github-starred/vikunja
2
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-05-06 19:47:47 -05:00
Code Issues 690 Packages Projects Releases 11 Wiki Activity

[PR #2210] [MERGED] feat: Idiomatic Go test behavior with Magefile #9834

New Issue
Closed
opened 2026-04-23 09:14:17 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-23 09:14:17 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/go-vikunja/vikunja/pull/2210
Author: @JohnStarich
Created: 2/9/2026
Status: ✅ Merged
Merged: 2/17/2026
Merged by: @kolaente

Base: main ← Head: feature/idiomatic-tests

📝 Commits (10+)

  • 87ffa50 style: run gofmt -s to update octal literals
  • da11334 refactor: remove environment variable requirements for go test
  • efafc1b style: fix doc comments to match godoc style
  • 5f7a9a9 refactor: remove root path in favor of Magefile default directory
  • 2e990b0 fix: add missing error checks in filepath.Walk and defer Close locations
  • 1c076eb feat: toggle test verbosity based on Mage verbose flag
  • ad06350 refactor: return errors to Mage instead of os.Exit and stream to stdout/stderr
  • 50c1050 refactor: switch to native filepath.Walk for gofmt file discovery
  • b05057a refactor: use Go idioms for running tests
  • d9f46fb fix: replace stray panic with return err

📊 Changes

14 files changed (+204 additions, -222 deletions)

View changed files

📝 magefile.go (+153 -194)
📝 pkg/db/test.go (+0 -2)
📝 pkg/db/test_fixtures.go (+8 -3)
📝 pkg/models/main_test.go (+0 -2)
📝 pkg/modules/migration/main_test.go (+0 -2)
📝 pkg/modules/migration/todoist/main_test.go (+3 -0)
📝 pkg/modules/migration/todoist/todoist_test.go (+1 -4)
📝 pkg/modules/migration/trello/main_test.go (+3 -0)
📝 pkg/modules/migration/trello/trello_test.go (+1 -5)
📝 pkg/modules/migration/vikunja-file/main_test.go (+0 -2)
📝 pkg/modules/migration/vikunja-file/vikunja_test.go (+2 -3)
📝 pkg/notifications/main_test.go (+0 -2)
📝 pkg/webtests/integrations.go (+1 -3)
➕ pkg/webtests/main_test.go (+32 -0)

📄 Description

I started work on urgency / weighted sort for tasks and ran into some surprises while building, testing, and linting:

  • go test ./... did not run for me successfully without additional runtime environment variables. I fixed this by go:embeding test fixtures and initializing config variables where necessary.
    • Please spot check me here! This one I had a hard time spotting usage of globals across tasks and package inits. I think I got them all.
    • Both Mage and go test implicitly set the current working directory before running tasks or testing packages. I took advantage of this to simplify file paths.
  • With go test busted for me, I tried Mage. Mage test tasks always resulted in high verbosity test output, which was difficult to read when identifying just failing test results. I've now linked Mage's verbosity to the test verbosity, so we can run mage -v to enable when desired.
  • Some common task failures resulted in os.Exit(), rather than returning contextual errors in tests or other mage tasks. This made troubleshooting pretty tricky. I returned errors instead and deferred print and exit logic to Mage.
  • Some mage tasks performed special go list all and find calls to carefully select sections of packages and files. This worked, but also tested all sorts of packages outside the current Go module, like standard library packages.
    • For the package lists, this appeared to only split by (predominantly) faster tests and slower tests. There's an app for that! I used go test -short ./... to only run faster tests, and left the single package selected for slow tests to run on their own.
    • For the file lists, I switched to a Go native filepath.Walk to pick up on the same files without the slightly less obvious syntax of find. This one wasn't too big a deal, find is pretty common.

Surprises aren't always bad, and this may totally be intended behavior, but I wanted to run this set of changes by you all. I believe adopting some common Go idioms like these will ease others' contributions as well as my own.

Feel free to reject this in part or as a whole. I carefully split my commits for a piecemeal revert if we need it or to more easily split into separate, smaller PRs. Just give the word, I'll hack it into smaller bits!

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/go-vikunja/vikunja/pull/2210 **Author:** [@JohnStarich](https://github.com/JohnStarich) **Created:** 2/9/2026 **Status:** ✅ Merged **Merged:** 2/17/2026 **Merged by:** [@kolaente](https://github.com/kolaente) **Base:** `main` ← **Head:** `feature/idiomatic-tests` --- ### 📝 Commits (10+) - [`87ffa50`](https://github.com/go-vikunja/vikunja/commit/87ffa506eb025e91468afa322ff78ee5baa69e9e) style: run gofmt -s to update octal literals - [`da11334`](https://github.com/go-vikunja/vikunja/commit/da11334706f937ba785d0dd453a8422c983e8924) refactor: remove environment variable requirements for go test - [`efafc1b`](https://github.com/go-vikunja/vikunja/commit/efafc1b2cdf880fec620f8f16e9e786204237f20) style: fix doc comments to match godoc style - [`5f7a9a9`](https://github.com/go-vikunja/vikunja/commit/5f7a9a92c2271a85fa149b17265e3595f93e262c) refactor: remove root path in favor of Magefile default directory - [`2e990b0`](https://github.com/go-vikunja/vikunja/commit/2e990b0c6a9de4147ff8cb371b928191453df2fc) fix: add missing error checks in filepath.Walk and defer Close locations - [`1c076eb`](https://github.com/go-vikunja/vikunja/commit/1c076eb2ccc968d0ebca381b3e26fec362973245) feat: toggle test verbosity based on Mage verbose flag - [`ad06350`](https://github.com/go-vikunja/vikunja/commit/ad063501a04ce3c6e627ea4682b0f4fa0bc8072b) refactor: return errors to Mage instead of os.Exit and stream to stdout/stderr - [`50c1050`](https://github.com/go-vikunja/vikunja/commit/50c10501fae7f51ec2ef41df8d2c82b4c179b0a8) refactor: switch to native filepath.Walk for gofmt file discovery - [`b05057a`](https://github.com/go-vikunja/vikunja/commit/b05057a0716a109bf672c825bf7c3585c66072e2) refactor: use Go idioms for running tests - [`d9f46fb`](https://github.com/go-vikunja/vikunja/commit/d9f46fb6edcf820f77af166758afa1308a7b6e98) fix: replace stray panic with return err ### 📊 Changes **14 files changed** (+204 additions, -222 deletions) <details> <summary>View changed files</summary> 📝 `magefile.go` (+153 -194) 📝 `pkg/db/test.go` (+0 -2) 📝 `pkg/db/test_fixtures.go` (+8 -3) 📝 `pkg/models/main_test.go` (+0 -2) 📝 `pkg/modules/migration/main_test.go` (+0 -2) 📝 `pkg/modules/migration/todoist/main_test.go` (+3 -0) 📝 `pkg/modules/migration/todoist/todoist_test.go` (+1 -4) 📝 `pkg/modules/migration/trello/main_test.go` (+3 -0) 📝 `pkg/modules/migration/trello/trello_test.go` (+1 -5) 📝 `pkg/modules/migration/vikunja-file/main_test.go` (+0 -2) 📝 `pkg/modules/migration/vikunja-file/vikunja_test.go` (+2 -3) 📝 `pkg/notifications/main_test.go` (+0 -2) 📝 `pkg/webtests/integrations.go` (+1 -3) ➕ `pkg/webtests/main_test.go` (+32 -0) </details> ### 📄 Description I started work on [urgency / weighted sort](https://community.vikunja.io/t/introduce-configurable-urgency-mechanism-akin-to-taskwarrior/4007/5) for tasks and ran into some surprises while building, testing, and linting: * `go test ./...` did not run for me successfully without additional runtime environment variables. I fixed this by `go:embed`ing test fixtures and initializing config variables where necessary. - Please spot check me here! This one I had a hard time spotting usage of globals across tasks and package inits. I _think_ I got them all. - Both Mage and `go test` implicitly set the current working directory before running tasks or testing packages. I took advantage of this to simplify file paths. * With `go test` busted for me, I tried Mage. Mage test tasks always resulted in high verbosity test output, which was difficult to read when identifying just failing test results. I've now linked Mage's verbosity to the test verbosity, so we can run `mage -v` to enable when desired. * Some common task failures resulted in `os.Exit()`, rather than returning contextual errors in tests or other mage tasks. This made troubleshooting pretty tricky. I returned errors instead and deferred print and exit logic to Mage. * Some mage tasks performed special `go list all` and `find` calls to carefully select sections of packages and files. This worked, but also tested all sorts of packages outside the current Go module, like standard library packages. - For the package lists, this appeared to only split by (predominantly) faster tests and slower tests. There's an app for that! I used `go test -short ./...` to only run faster tests, and left the single package selected for slow tests to run on their own. - For the file lists, I switched to a Go native `filepath.Walk` to pick up on the same files without the slightly less obvious syntax of `find`. This one wasn't too big a deal, `find` is pretty common. Surprises aren't always bad, and this may totally be intended behavior, but I wanted to run this set of changes by you all. I believe adopting some common Go idioms like these will ease others' contributions as well as my own. Feel free to reject this in part or as a whole. I carefully split my commits for a piecemeal revert if we need it or to more easily split into separate, smaller PRs. Just give the word, I'll hack it into smaller bits! --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-23 09:14:17 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/api
area/attachments
area/auth
area/avatars
area/backup-restore
area/caldav
area/calendar-view
area/comments
area/config
area/database
area/desktop
area/docker
area/email
area/favorites
area/filters
area/frontend
area/gantt
area/i18n
area/import-export
area/internal-code
area/kanban
area/labels
area/list-view
area/mobile
area/notifications
area/permissions
area/projects
area/pwa
area/recurring-tasks
area/reminders
area/search
area/shortcuts
area/subtasks
area/sync
area/table-view
area/task-editor
area/task-metadata
area/task-relations
area/teams
area/theming
area/time-tracking
area/typesense
area/views
area/webhooks
bug
changes requested
concern/accessibility
concern/performance
concern/regression
concern/ux
confirmed
db/mysql
dependencies
enhancement
good first issue
help wanted
integration/inbound
integration/outbound
kind/bug
kind/feature
needs reproduction
pull-request
Mirrored from GitHub Pull Request
 question
security
support
upstream issue
waiting for reply
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/vikunja#9834
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?