Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix storage_test suite #28

Closed
wants to merge 1 commit into from
Closed

Conversation

kula
Copy link

@kula kula commented Oct 2, 2022

  • add ctx parameter to ConsulStorage function calls which now require it
  • Set cs.Prefix properly --- setting the EnvNamePrefix environment variable does not work in the tests because that is only read if the module is be instantiated by Caddy, not during the tests. This not only actually uses the correct prefix, but now the KV cleanup done in setupConsulEnv cleans the proper path, so you can run tests multiple times in a row without errors because random K/V entries are left
  • Set the logger in setupConsulEnv
  • Switch from certmagic.ErrNotExist to fs.ErrNotExist --- see Any error satisfies ErrNotExist caddyserver/certmagic#168

- add `ctx` parameter to ConsulStorage function calls which
  now require it
- Set cs.Prefix properly --- setting the EnvNamePrefix environment
  variable does not work in the tests because that is only read if
  the module is be instantiated by Caddy, not during the tests. This
  not only actually uses the correct prefix, but now the KV cleanup
  done in `setupConsulEnv` cleans the proper path, so you can run
  tests multiple times in a row without errors because random K/V
  entries are left
- Set the logger in `setupConsulEnv`
- Switch from certmagic.ErrNotExist to fs.ErrNotExist ---
  see caddyserver/certmagic#168
@kula kula mentioned this pull request Oct 2, 2022
@pteich
Copy link
Owner

pteich commented Sep 10, 2024

Fixed alongside with #32
Also switched to testcontainers to make it easier to run tests.

@pteich pteich closed this Sep 10, 2024
This pull request was closed.
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.

2 participants