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

Don't compile windows and windows-sys in unit test mode #3112

Merged
merged 2 commits into from
Jun 19, 2024

Conversation

sivadeilra
Copy link
Collaborator

This PR improves cycle time for running cargo test --all.

The windows and windows-sys crates do not contain unit tests or doc tests. Cargo doesn't know that, so it compiles these two very large crates in the #[cfg(test)] mode and wastes time scanning for doc tests.

This PR simply disables unit tests and doc tests in both of these crates. This saves a significant amount of time in running tests.

@kennykerr
Copy link
Collaborator

Should we be seeing a reduction here? https://github.com/microsoft/windows-rs/actions/workflows/test.yml

@sivadeilra
Copy link
Collaborator Author

Should we be seeing a reduction here? https://github.com/microsoft/windows-rs/actions/workflows/test.yml

I certainly see a reduction locally.

@kennykerr
Copy link
Collaborator

Can you illustrate? I just don't see cargo test doing much for the windows crate unless you explicitly add --all-features.

@sivadeilra
Copy link
Collaborator Author

Can you illustrate? I just don't see cargo test doing much for the windows crate unless you explicitly add --all-features.

What I see happening is that we spend a bunch of time compiling the windows crate in test mode, even though it doesn't contain any tests. Then we "run" it and of course that takes no time, but we spent a long time compiling a useless executable.

I see this happening when I run cargo test from the repo root. When you run cargo test directly on the windows crate you won't see it because no features are activated.

@kennykerr
Copy link
Collaborator

Ah, strangely this affects 'cargo test' but not 'cargo test --all' which is what I've trained myself to do. 🤷‍♂️

@kennykerr kennykerr self-requested a review June 19, 2024 12:54
Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

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

There aren't any tests but there are docs.

@kennykerr kennykerr merged commit f6f985d into microsoft:master Jun 19, 2024
89 checks passed
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.

3 participants