Skip to content

Suggest adding a #[cfg(test)] to to a test module#91770

Merged
bors merged 1 commit intorust-lang:masterfrom
TaKO8Ki:suggest-adding-cfg-test
Dec 21, 2021
Merged

Suggest adding a #[cfg(test)] to to a test module#91770
bors merged 1 commit intorust-lang:masterfrom
TaKO8Ki:suggest-adding-cfg-test

Conversation

@TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Dec 11, 2021

closes #88138

@rust-highfive
Copy link
Contributor

rust-highfive commented Dec 11, 2021

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2021
@TaKO8Ki TaKO8Ki changed the title Suggest adding #[cfg(test)] to to a test module Suggest adding a #[cfg(test)] to to a test module Dec 11, 2021
camelid
camelid reviewed Dec 12, 2021
View reviewed changes
src/test/ui/imports/unused-imports-in-test-module.stderr Outdated
Comment on lines 19 to 23
Copy link
Member

@camelid camelid Dec 12, 2021

Choose a reason for hiding this comment

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

Seems like it'd be good to add tests for out-of-line modules too (i.e,, mod test;). But I'm not sure how feasible it is to test that.

@camelid camelid added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 13, 2021
joshtriplett
joshtriplett reviewed Dec 15, 2021
View reviewed changes
@joshtriplett
Copy link
Member

joshtriplett commented Dec 15, 2021

This looks great to me. I posted one comment about the proposed diagnostic message; with that changed, r=me.

@TaKO8Ki TaKO8Ki force-pushed the suggest-adding-cfg-test branch from e343965 to 4103248 Compare December 15, 2021 14:49
@camelid
Copy link
Member

camelid commented Dec 15, 2021

Also, please squash your commits if you can :)

@TaKO8Ki
suggest adding a #[cfg(test)] to test modules
6f8ad6d 
remove a empty line

import `module_to_string`

use `contains("test")`

show a suggestion in case module starts_with/ends_with "test"

replace `parent` with `containing`
@TaKO8Ki TaKO8Ki force-pushed the suggest-adding-cfg-test branch from 4103248 to 6f8ad6d Compare December 16, 2021 02:49
@jackh726
Copy link
Member

jackh726 commented Dec 20, 2021

@bors r=joshtriplett

@bors
Copy link
Collaborator

bors commented Dec 20, 2021

📌 Commit 6f8ad6d has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 20, 2021
@bors
Copy link
Collaborator

bors commented Dec 20, 2021

⌛ Testing commit 6f8ad6d with merge e6c58674ebb4b3aeded006ee47526f6ec77d97e0...

@bors
Copy link
Collaborator

bors commented Dec 20, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 20, 2021
@matthiaskrgr
Copy link
Member

matthiaskrgr commented Dec 20, 2021

@bors retry docker.io 503

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 20, 2021
@rust-log-analyzer
Copy link
Collaborator

rust-log-analyzer commented Dec 21, 2021

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
Attempting with retry: docker build --rm -t rust-ci -f /gha/_work/rust/rust/src/ci/docker/host-aarch64/aarch64-gnu/Dockerfile /gha/_work/rust/rust/src/ci/docker
Sending build context to Docker daemon  506.4kB

Step 1/6 : FROM ubuntu:20.04
Head https://registry-1.docker.io/v2/library/ubuntu/manifests/20.04: received unexpected HTTP status: 503 Service Unavailable
Sending build context to Docker daemon  506.4kB

Step 1/6 : FROM ubuntu:20.04
Step 1/6 : FROM ubuntu:20.04
Head https://registry-1.docker.io/v2/library/ubuntu/manifests/20.04: received unexpected HTTP status: 503 Service Unavailable
Sending build context to Docker daemon  506.4kB

Step 1/6 : FROM ubuntu:20.04
Step 1/6 : FROM ubuntu:20.04
Head https://registry-1.docker.io/v2/library/ubuntu/manifests/20.04: received unexpected HTTP status: 503 Service Unavailable
Sending build context to Docker daemon  506.4kB

Step 1/6 : FROM ubuntu:20.04
Step 1/6 : FROM ubuntu:20.04
Head https://registry-1.docker.io/v2/library/ubuntu/manifests/20.04: received unexpected HTTP status: 503 Service Unavailable
Sending build context to Docker daemon  506.4kB

Step 1/6 : FROM ubuntu:20.04
Step 1/6 : FROM ubuntu:20.04
Head https://registry-1.docker.io/v2/library/ubuntu/manifests/20.04: received unexpected HTTP status: 503 Service Unavailable
##[error]Process completed with exit code 1.
Post job cleanup.

@matthiaskrgr matthiaskrgr mentioned this pull request Dec 21, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 21, 2021
@bors
Auto merge of rust-lang#92152 - matthiaskrgr:rollup-nmskpw6, r=matthi…
3d57c61 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90345 (Stabilise entry_insert)
 - rust-lang#91412 (Improve suggestions for importing out-of-scope traits reexported as `_`)
 - rust-lang#91770 (Suggest adding a `#[cfg(test)]` to to a test module)
 - rust-lang#91823 (Make `PTR::as_ref` and similar methods `const`.)
 - rust-lang#92127 (Move duplicates removal when generating results instead of when displaying them)
 - rust-lang#92129 (JoinHandle docs: add missing 'the')
 - rust-lang#92131 (Sync rustc_codegen_cranelift)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 790950a into rust-lang:master Dec 21, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 21, 2021
@TaKO8Ki TaKO8Ki deleted the suggest-adding-cfg-test branch December 22, 2021 04:53
@ken-matsui ken-matsui mentioned this pull request May 2, 2022
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 17, 2022
@JohnTitor
Rollup merge of rust-lang#96651 - ken-matsui:omit-unnecessary-help-to…
78c709c 
…-add-cfg-test, r=cjgillot

Omit unnecessary help to add `#[cfg(test)]` when already annotated

Closes: rust-lang#96611

The related PR is: rust-lang#91770
@TaKO8Ki TaKO8Ki mentioned this pull request Jul 8, 2022
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 17, 2022
@JohnTitor
Rollup merge of rust-lang#99036 - TaKO8Ki:fix-test-for-88138, r=compi…
0748638 
…ler-errors

Add `#[test]` to functions in test modules

I implemented a suggestion in rust-lang#91770, but the ui test I created was inadequate and I have fixed that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshtriplett joshtriplett Awaiting requested review from joshtriplett

@camelid camelid Awaiting requested review from camelid

Assignees

@matthewjasper matthewjasper

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.59.0

Development

Successfully merging this pull request may close these issues.

Unused import warning in a test could suggest #[cfg(test)]

10 participants

@TaKO8Ki @rust-highfive @joshtriplett @camelid @jackh726 @bors @matthiaskrgr @rust-log-analyzer @matthewjasper @rustbot