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

Split integration test into multiple files #91

Merged
merged 3 commits into from
May 1, 2019

Conversation

KSXGitHub
Copy link
Contributor

@KSXGitHub KSXGitHub commented Apr 28, 2019

Fix #85

An additional issue arises: cargo test warns me of unused items in helper module. This happens because the items are only used by test functions in test-only context. I don't know how to fix this other than #[allow(dead_code)].

@svenstaro
Copy link
Owner

svenstaro commented Apr 28, 2019 via email

@KSXGitHub
Copy link
Contributor Author

Can't you properly fix the warnings using cfg(test)?

How exactly am I going to do this? I mean, if they are all in a single file, I can wrap them in a #[cfg(test)] mod, but they're scattered in multiple files.

@ghost
Copy link

ghost commented Apr 28, 2019

You could move the files into a subfolder of tests, call it integration or whatever you think is a good name. It should solve the unused code warnings.

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented Apr 29, 2019

@boastful-squirrel Your suggestion also hides dead code warnings, are you sure it is not #[allow(dead_code) with extra steps?

For reference: Here's what I did

@svenstaro
Copy link
Owner

According to the official guide, you're supposed to make it tests/helpers/mod.rs instead of tests/helpers.rs.

@svenstaro
Copy link
Owner

Also please call it fixtures.rs as rstest mimicks pytest and there it's common to call this sort of thing fixtures.

@KSXGitHub
Copy link
Contributor Author

@svenstaro Is this what you suggested?

filesystem tree

Warnings didn't go anywhere:

terminal output

@svenstaro
Copy link
Owner

Looks exactly like rust-lang/rust#46379

Putting it under tests/fixtures/mod.rs does have one benefit though: It makes cargo stop trying to run it as a test.

@svenstaro
Copy link
Owner

The workaround mentioned in the issue works btw but it does hide real dead code. Overall I'm not happy with this and I'm not sure what the best solution would be.

@KSXGitHub
Copy link
Contributor Author

Putting it under tests/fixtures/mod.rs does have one benefit though: It makes cargo stop trying to run it as a test.

I believe not marking a function #[test] (or #[rstest]) makes cargo stop trying to run it as a test.

@KSXGitHub
Copy link
Contributor Author

I'm not sure what the best solution would be.

The best solution at the moment in my opinion is to #[allow(dead_code)]

@svenstaro
Copy link
Owner

Putting it under tests/fixtures/mod.rs does have one benefit though: It makes cargo stop trying to run it as a test.

I believe not marking a function #[test] (or #[rstest]) makes cargo stop trying to run it as a test.

True, but sadly it doesn't make cargo stop compiling the file. For that, you'll need to do the mod.rs trick.

I agree, for now, using #[allow(dead_code)] is the best choice. But please document why you use that somewhere in the file and maybe link the cargo bug report.

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented Apr 30, 2019

True, but sadly it doesn't make cargo stop compiling the file. For that, you'll need to do the mod.rs trick.

Would writing a #[test] function in mod.rs disproves your claim? (I haven't try it yet)

@svenstaro
Copy link
Owner

True, but sadly it doesn't make cargo stop compiling the file. For that, you'll need to do the mod.rs trick.

Would writing a #[test] function in mod.rs disproves your claim? (I haven't try it yet)

Well, it's not my claim, it's what's written in the docs. You can try, though. Let's get this merged.

@KSXGitHub
Copy link
Contributor Author

Let's get this merged.

Not yet.

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented May 1, 2019

I have moved helpers.rs to fixtures/mod.rs for organizational purpose only, not for cargo test to ignore the file (it does not, I just tested it).

@svenstaro Please review

@svenstaro
Copy link
Owner

svenstaro commented May 1, 2019

Actually putting stuff into fixtures/mod.rs does work exactly as expected. Compare the output of cargo test.

cargo test with tests/fixtures.rs:

test result: ok. 15 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/auth-f85dbcb4ad30be17

running 3 tests
test auth_works::case_3 ... ok
test auth_works::case_1 ... ok
test auth_works::case_2 ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/cli-13adc5119dab8f54

running 2 tests
test version_shows ... ok
test help_shows ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/fixtures-a0fdb7b285082d65

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/serve_request-86b061418b6f5951

running 2 tests
test serves_requests_with_no_options ... ok
test serves_requests_with_non_default_port ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/upload_files-b935706e952a427b

running 1 test
test uploading_files_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

cargo test with tests/fixtures/mod.rs:

test result: ok. 15 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/auth-f85dbcb4ad30be17

running 3 tests
test auth_works::case_2 ... ok
test auth_works::case_1 ... ok
test auth_works::case_3 ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/cli-13adc5119dab8f54

running 2 tests
test version_shows ... ok
test help_shows ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/serve_request-86b061418b6f5951

running 2 tests
test serves_requests_with_no_options ... ok
test serves_requests_with_non_default_port ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running target/debug/deps/upload_files-b935706e952a427b

running 1 test
test uploading_files_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Since you're saying that you cannot reproduce this, what cargo version are you using? It might be worth reporting this upstream if this is a regression.

@svenstaro svenstaro merged commit f099134 into svenstaro:master May 1, 2019
@KSXGitHub KSXGitHub deleted the split-integration-test branch May 1, 2019 06:25
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.

Divide tests/cli.rs into multiple files
2 participants