-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
caddytest: internalize init config into '.go' file #5230
Conversation
@greenpau can you check this PR by depending on the branch |
@mohammed90 , thank you for looking into it. Added the following to
Ran:
I could see the test itself succeeds, b/c it gets to lines 59-60. https://github.com/greenpau/caddy-security/blob/cfc03c5f47313f4b2d6699193659f41040511b49/app_test.go#L59-L60
But then
The config I am using is here https://github.com/greenpau/caddy-security/blob/cfc03c5f47313f4b2d6699193659f41040511b49/assets/config/Caddyfile |
Your Caddyfile overrides the admin address, changing it from 2999 on first POST to 2019. That issue did occur to me, but I didn't figure out a solution to how to inject the correct admin port in client config. For now, just include |
added @mohammed90 , the test passes now! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great find! @mohammed90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mohammed90, nice workaround!
Go module distribution excludes files that are not of a specific list of types. In other words,
.go
files are retained in the downloaded modules, but others are not for sure1. This causes pain, e.g. #5187. The workaround in this PR is to write the init config to a temp file, and set the cleanup func to delete the file at the end of the test run.For the record, I tried passing the init config t o
os.Stdin
by creating aos.Pipe
, writing to the writer end, and settingos.Stdin = reader
. However, this seems to slow down the run to the extent of test timeouts. It did work sometimes when the timeout value is set for very long period. It's probably a race condition. I don't believe forcefully going down that route is worth it. Writing the init config to a temp file is much simpler and easier to maintain, unless I'm blind to some aspect.Updates #5187
Footnotes
I couldn't find a definite list of excluded files. If any knows of a reference, please share it! ↩