Skip to content

Detect integration tests in is_in_test - #16786

Merged
rustbot merged 1 commit into
rust-lang:masterfrom
ClementTsang:allow-unwrap-in-integration-tests
Sep 7, 2026
Merged

Detect integration tests in is_in_test#16786
rustbot merged 1 commit into
rust-lang:masterfrom
ClementTsang:allow-unwrap-in-integration-tests

Conversation

@ClementTsang

@ClementTsang ClementTsang commented Mar 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related: #13981

This PR makes it so that the is_in_test check accounts for if a node is in an integration test file (that is, under tests/). This should fix the issue where some clippy settings work for cfg(test) but not for integration tests.

This could be extended to benches as well if this seems reasonable.

Note that this is the first time I'm contributing to this repo, so please let me know if there's anything to change. I'm not really sure how I was meant to test this for example, especially if I want to test an example of this where it should still "fail", I guess (should I split the integration_test with two crates?).

changelog: [unwrap-used]: allow-unwrap-in-tests works now on integration tests.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 31, 2026
@rustbot

rustbot commented Mar 31, 2026

Copy link
Copy Markdown
Collaborator

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@ClementTsang
ClementTsang marked this pull request as draft March 31, 2026 05:51
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 31, 2026
@ClementTsang
ClementTsang force-pushed the allow-unwrap-in-integration-tests branch from d96fa88 to e3772c6 Compare March 31, 2026 05:55
@ClementTsang

ClementTsang commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Hmmm it looks like it's causing problems because now some of clippy's tests are being treated as valid under is_in_test... let me set this to draft for now and figure this out. For whatever reason I didn't catch this when testing locally earlier.

@ClementTsang
ClementTsang force-pushed the allow-unwrap-in-integration-tests branch from 10d22da to 07033e9 Compare April 3, 2026 08:33
@ClementTsang
ClementTsang marked this pull request as ready for review April 3, 2026 08:33
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 3, 2026
@ClementTsang
ClementTsang force-pushed the allow-unwrap-in-integration-tests branch from 4588d65 to d234340 Compare May 7, 2026 04:01
@rustbot

This comment has been minimized.

Sign in to view
@ClementTsang ClementTsang mentioned this pull request May 8, 2026
Open
@blyxyas

blyxyas commented Jun 14, 2026

Copy link
Copy Markdown
Member

Hello @ClementTsang, I hope that you don't mind we using this contribution to teach @medzernik some things about reviewing (they were very interested!).

cc @medzernik Check out this comment first.

@ClementTsang

ClementTsang commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

@blyxyas nope, I don't mind!

@medzernik

medzernik commented Jun 14, 2026

Copy link
Copy Markdown

r? @medzernik

@rustbot rustbot assigned medzernik and unassigned dswij Jun 14, 2026
medzernik
medzernik approved these changes Jul 3, 2026
View reviewed changes

@medzernik medzernik left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I didn't find anything of note.. I guess approve? :D

View changes since this review

@blyxyas

blyxyas commented Jul 31, 2026

Copy link
Copy Markdown
Member

I'm going to review this tomorrow, sorry for the radio silence ( =ノωヽ=) (I'm finally back up!)

@ClementTsang

ClementTsang commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

No worries, thanks to both of you for taking the time to review it!

blyxyas
blyxyas requested changes Aug 28, 2026
View reviewed changes

@blyxyas blyxyas left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good, just a couple of asks!!

View changes since this review

Comment thread tests/example_integration_test/Cargo.toml
Comment thread tests/allow-unwrap-in-integration-tests.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 28, 2026
@rustbot

rustbot commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@ClementTsang
ClementTsang requested a review from blyxyas August 29, 2026 06:06
@Jarcho Jarcho mentioned this pull request Aug 30, 2026
Closed
blyxyas
blyxyas requested changes Aug 31, 2026
View reviewed changes

@blyxyas blyxyas left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Everything else is looking great! Please merge those two functions and squash the commits, then we can look again (and hopefully) approve! ฅ՞•ﻌ•՞ฅ

View changes since this review

Comment thread tests/compile-test.rs Outdated
@ClementTsang
ClementTsang force-pushed the allow-unwrap-in-integration-tests branch from d8b6122 to 9017078 Compare September 1, 2026 08:28
@rustbot

This comment has been minimized.

Sign in to view
@ClementTsang
ClementTsang force-pushed the allow-unwrap-in-integration-tests branch from 9017078 to 6d5ec6c Compare September 1, 2026 08:39
@rustbot

rustbot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@ClementTsang
ClementTsang requested a review from blyxyas September 1, 2026 08:39
@ClementTsang
ClementTsang force-pushed the allow-unwrap-in-integration-tests branch 2 times, most recently from 237c62b to 44e73f6 Compare September 1, 2026 08:54
@ClementTsang
Detect integration tests in is_in_test
1ea7460 
Makes it so that the is_in_test check accounts for if a node is
in an integration test file (that is, under tests/). This should
fix the issue where some clippy settings work for cfg(test) but
not for integration tests.
@ClementTsang
ClementTsang force-pushed the allow-unwrap-in-integration-tests branch from 44e73f6 to 1ea7460 Compare September 1, 2026 09:08
blyxyas
blyxyas approved these changes Sep 7, 2026
View reviewed changes

@blyxyas blyxyas left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for addressing this issue! ❤️ ฅ•ω•ฅ And welcome to the project!

View changes since this review

@blyxyas
blyxyas added this pull request to the merge queue Sep 7, 2026
@blyxyas
blyxyas removed this pull request from the merge queue due to a manual request Sep 7, 2026
@blyxyas

blyxyas commented Sep 7, 2026

Copy link
Copy Markdown
Member

@rustbot merge

@rustbot
rustbot added this pull request to the merge queue Sep 7, 2026
Merged via the queue into rust-lang:master with commit 13aece1 Sep 7, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blyxyas blyxyas blyxyas approved these changes
+1 more reviewer
@medzernik medzernik medzernik approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@medzernik medzernik

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ClementTsang @rustbot @blyxyas @medzernik @dswij