Skip to content

fix usage of undefined env. var in itests#791

Open
rursprung wants to merge 1 commit intogoogle:mainfrom
rursprung:fix-itest-path-lookup
Open

fix usage of undefined env. var in itests#791
rursprung wants to merge 1 commit intogoogle:mainfrom
rursprung:fix-itest-path-lookup

Conversation

@rursprung
Copy link
Copy Markdown

so far, the integration tests relied on the CARGO_TARGET_DIR env. variable. this usage was wrong since the variable is not actually set, so the fallback "target" was always used.

CARGO_MANIFEST_DIR is always set and we know that the target directory is one level up from there, so we can use that instead. std::path::Path is used to ensure that the correct path separator is used.

the run_integration_tests.sh script is not affected by this since it is meant to be launched from the project root directory and then the path constructed so far already worked. but if running the tests e.g. from an IDE (after compiling the itest binaries) then the working directory will be in the integration_tests subfolder and the constructed path will be wrong (this is how i initially noticed this bug).

so far, the integration tests relied on the `CARGO_TARGET_DIR` env.
variable. this usage was wrong since the variable is not actually set,
so the fallback `"target"` was always used.

`CARGO_MANIFEST_DIR` is always set and we know that the target directory
is one level up from there, so we can use that instead.
`std::path::Path` is used to ensure that the correct path separator is
used.

the `run_integration_tests.sh` script is not affected by this since it
is meant to be launched from the project root directory and then the
path constructed so far already worked. but if running the tests e.g.
from an IDE (after compiling the itest binaries) then the working
directory will be in the `integration_tests` subfolder and the
constructed path will be wrong (this is how i initially noticed this
bug).
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.

1 participant