Allow environment variables in #[files] attributes #277
Conversation
The environment variable must be passed to cargo test. Any variable name is valid and would be replaced. If a variable is not defined, the macro will error with the proper error message.
|
I changed the test to add a Here is the result when running with the environment variable undeclared:
Here is the same test with a proper value (just having fun doing up and down on path):
Please note that if the environment variable is declared but doesn't go into the proper folder, it is equivalent to specifying the wrong path in |
There was a problem hiding this comment.
Really nice JOB!!! Thanks for your contribution.
Can you please add some unit tests, a line in change log and add document this new feature in
https://github.com/la10736/rstest/blob/master/rstest/src/lib.rs#L954 and https://github.com/la10736/rstest/blob/master/README.md?plain=1#L233 ?
And other things, like documentation and changelog.
|
@la10736 Thanks for the review. Added a new PTAL |
To run the tests, you require adding the `wpt-tests` feature and setting the `WPT_ROOT` environment variable on the command line. This is currently depending on a pending rstest PR: la10736/rstest#277 The current set of curated tests are all working.
There was a problem hiding this comment.
That's an amazing work. Thanks again.
I would have preferred that formatting was separated by code changing and some more unit tests about parsing: but I really appreciated your work and I should find a compromise with my desiderata 😄
I'm not sure if I really like the base_dir feature but maybe it can be useful in some context.
|
Just a note about tests and coverage: pass in every lines (100% coverage) doesn't means spot all issues.... especially on on algorithms where the corner cases are not trivially identified by if branches. Anyway, your code and tests are really great! |
| // Make sure cargo would rerun if the value of this environment variable changes. | ||
| println!("cargo::rerun-if-env-changed={var_name}"); |
There was a problem hiding this comment.
Unfortunately it doesn't seem to work. I tried it this feature for base root and if just change, add or remove this variable the tests are not recompiled. If I add if to a build script it seams to work.
I'll remove it and add some documentation notes.
There was a problem hiding this comment.
I tested it on my side, let me investigate a bit.
The environment variable must be passed to cargo test. Any variable name is valid and would be replaced. If a variable is not defined, the macro will error with the proper error message.
Fixes #276