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 test commands #24

Merged
merged 1 commit into from
Jun 24, 2023
Merged

Conversation

KoltesDigital
Copy link
Contributor

The path was transformed twice, which seems not to be a problem on Linux, but it is on Windows.

thread 'cli_tests::basic::it_process_toml_to_stdout' panicked at 'called `Result::unwrap()` on an `Err` value: CargoError { cause: Some(NotFoundError { path: "XYZ\\tera-cli\\target\\debug\\tera.exe.exe" }) }', tests\test.rs:46:74

@chevdor
Copy link
Owner

chevdor commented Jun 23, 2023

I don't think this PR is a good idea. We do not want to use tera in the tests but the local binary produced by the build.

@chevdor
Copy link
Owner

chevdor commented Jun 23, 2023

That being said, I see your problem in tera.exe.exe. I don't have a windows machine but it sounds you do.
Could you test if using CARGO_BIN_tera works better on windows ?

Copy link
Owner

@chevdor chevdor left a comment

Choose a reason for hiding this comment

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

The proposed fix is not good.
Try uninstalling tera from your system and running the tests...

@KoltesDigital
Copy link
Contributor Author

I think you've read too quickly. You used both Command::cargo_bin and CARGO_BIN_EXE_xxx, whose purpose is the same, and that's why there are two .exe. I've just removed the latter, so I'm still relying on Command::cargo_bin("tera") to result in "XYZ\tera-cli\target\debug\\tera.exe". It is not Command::new("tera"). On the machine where I coded this PR I have no global tera in PATH.

I could have used env!("CARGO_BIN_EXE_tera") instead, it's supposed to work too, but I preferred the former because it explicitly calls cargo_bin so there's the semantics of an action going through Cargo.toml, whereas the env var is relying on some external deity to set up the environment beforehand. But that's debatable.

Copy link
Owner

@chevdor chevdor left a comment

Choose a reason for hiding this comment

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

Yes you are right indeed.

@chevdor chevdor merged commit 713333f into chevdor:master Jun 24, 2023
@KoltesDigital KoltesDigital deleted the fix-test-command branch June 24, 2023 10:15
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