Skip to content

Conversation

@brandur
Copy link
Collaborator

@brandur brandur commented Aug 10, 2025

Follows up #390 with a couple tweaks for the tests:

  • Make auth middleware tests more focused so they only test
    authMiddleware rather than the full server HTTP stack.

  • Remove use of t.Setenv so we can run all tests in parallel.

  • Use pathPrefix in middleware so that only the specific configured
    prefix is accepted rather than any prefix that might be present.

  • Use CamelCaseTestName convention rather than "test name like this".

  • Use testBundle convention for setup.

Follows up #390 with a couple tweaks for the tests:

* Make auth middleware tests more focused so they only test
  `authMiddleware` rather than the full server HTTP stack.

* Remove use of `t.Setenv` so we can run all tests in parallel.

* Use `pathPrefix` in middleware so that only the specific configured
  prefix is accepted rather than any prefix that might be present.

* Use `CamelCaseTestName` convention rather than "test name like this".

* Use `testBundle` convention for `setup`.
Copy link
Contributor

@bgentry bgentry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I think this is all gonna conflict with the large PR #379 and I already did one round of fixes for the prior related change. Maybe this can wait until after it's merged to work this one in since it's small?

@brandur
Copy link
Collaborator Author

brandur commented Aug 14, 2025

Yeah that should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants