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

internal tests launch executable on every test run #5965

Closed
matthiaskrgr opened this issue Sep 3, 2018 · 5 comments · Fixed by #7576
Closed

internal tests launch executable on every test run #5965

matthiaskrgr opened this issue Sep 3, 2018 · 5 comments · Fixed by #7576
Labels
A-testing-cargo-itself Area: cargo's tests

Comments

@matthiaskrgr
Copy link
Member

There is something strange in the cargo tests.
On every run, it launches a third party program (calibre) to open some documents.
To reproduce, run cargo test workspace_open in the cargo repo.
The behavior is caused by these tests:

doc::doc_workspace_open_different_library_and_package_names
doc::doc_workspace_open_binary
doc::doc_workspace_open_binary_and_library

This is annoying when running the test suit because I have to close 3 windows after every run.

@matthiaskrgr matthiaskrgr changed the title internal tests launches executable on every test run internal tests launch executable on every test run Sep 3, 2018
@matthiaskrgr
Copy link
Member Author

Probably caused by cargo doc --open

p.cargo("doc --all --open")

cc @dwijnand

@dwijnand
Copy link
Member

dwijnand commented Sep 3, 2018

I touched that line last for test suite cleanup reasons, @debris is the original author (#5024).

Perhaps this behaviour is due to the change to --open in #5888 by @Seeker14491?

@Seeker14491
Copy link
Contributor

I see the problem.

cargo/tests/testsuite/doc.rs

Lines 1028 to 1032 in 9ab347d

p.cargo("doc --open")
.env("BROWSER", "echo")
.with_stderr_contains("[..] Documenting foo v0.1.0 ([..])")
.with_stderr_contains("[..] CWD/target/doc/foolib/index.html")
.run();

We set the BROWSER environment variable to echo, which would previously override what program cargo doc --open would use to open the generated docs. This behavior was removed with my changes in #5888; the xdg-open script is always used now, and completely handles finding a program. xdg-open does look at the BROWSER environment variable, but only as a fallback.

A note: the previous behavior of the BROWSER override was not implemented on Windows or Mac, which is why these tests all have the cfg

#[cfg(not(any(target_os = "windows", target_os = "macos")))]

I'm not sure what the best way to fix these tests would be. Even if the override were implemented, it really wouldn't be testing the normal operation of the --open flag, which is opening in the user's default browser.

@alexcrichton alexcrichton added the A-testing-cargo-itself Area: cargo's tests label Sep 4, 2018
@jonas-schievink
Copy link
Contributor

I just hit this too. I was really irritated when the test suite suddenly opened Firefox.

@ehuss
Copy link
Contributor

ehuss commented Sep 17, 2019

There is more discussion at #6064. I think cargo should be using the BROWSER env var if it is set.

bors added a commit that referenced this issue Nov 11, 2019
…excrichton

Add back support for `BROWSER` envvar in `cargo doc --open`.

Fixes #6064.
Fixes #5965.
@bors bors closed this as completed in 23a2efe Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing-cargo-itself Area: cargo's tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants