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

feat(test): Auto-redact elapsed time #13973

Merged
merged 1 commit into from
May 28, 2024
Merged

feat(test): Auto-redact elapsed time #13973

merged 1 commit into from
May 28, 2024

Conversation

epage
Copy link
Contributor

@epage epage commented May 28, 2024

By making this a redaction,
it automatically gets applied when generating the snapshot,
removing these extra steps
(that you likely only discover after the fact and have to debug)

@rustbot
Copy link
Collaborator

rustbot commented May 28, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
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

@rustbot rustbot added A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 28, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Regex support looks quite useful, but what's the benefit of this over [..] in this specific test case?

Copy link
Member

Choose a reason for hiding this comment

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

I should have asked this in the previous PR. I saw it as a dependency update so forgot to ask.

Copy link
Contributor Author

@epage epage May 28, 2024

Choose a reason for hiding this comment

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

When you first create a snapshot, there is no [..] so it might work for that run or for your machine but then fail. Or if you changed output near enough to a [..] you run into this when updating the snapshot.

By making this a redaction, it automatically gets applied when generating the snapshot, removing these extra steps (that you likely only discover after the fact and have to debug)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the clarification. Updated this in the PR description.

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 28, 2024

📌 Commit 5ea1c8f has been approved by weihanglo

It is now in the queue for this repository.

@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 May 28, 2024
@bors
Copy link
Contributor

bors commented May 28, 2024

⌛ Testing commit 5ea1c8f with merge 2b1e750...

bors added a commit that referenced this pull request May 28, 2024
feat(test): Auto-redact elapsed time

By making this a redaction,
it automatically gets applied when generating the snapshot,
removing these extra steps
(that you likely only discover after the fact and have to debug)
@bors
Copy link
Contributor

bors commented May 28, 2024

💔 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 May 28, 2024
@epage
Copy link
Contributor Author

epage commented May 28, 2024

@bors retry

@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 May 28, 2024
@bors
Copy link
Contributor

bors commented May 28, 2024

⌛ Testing commit 5ea1c8f with merge 2415192...

@bors
Copy link
Contributor

bors commented May 28, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 2415192 to master...

@bors bors merged commit 2415192 into rust-lang:master May 28, 2024
23 checks passed
@epage epage deleted the elapsed branch May 28, 2024 17:38
bors added a commit to rust-lang-ci/rust that referenced this pull request May 28, 2024
Update cargo

5 commits in a8d72c675ee52dd57f0d8f2bae6655913c15b2fb..431db31d0dbeda320caf8ef8535ea48eb3093407
2024-05-24 03:34:17 +0000 to 2024-05-28 18:17:31 +0000
- Include `lints.rust.unexpected_cfgs.check-cfg` in the fingerprint (rust-lang/cargo#13958)
- feat(test): Auto-redact elapsed time (rust-lang/cargo#13973)
- chore: Update to snapbox 0.6 (rust-lang/cargo#13963)
- fix: check if rev is full commit sha for github fast path (rust-lang/cargo#13969)
- test: switch from `drop` to `let _` due to nightly rustc change (rust-lang/cargo#13964)

r? ghost
@ehuss ehuss added this to the 1.80.0 milestone Jun 2, 2024
bors added a commit that referenced this pull request Jun 2, 2024
refactor: Transition direct assertions from cargo-test-support to snapbox

### What does this PR try to resolve?

Cargo has a bespoke testing framework for functional tests
- Extra stuff for us to maintain
- Don't leverage benefits from contributions related to other projects
- Less incentive to be thoroughly documented

UI tests are written using snapbox.  The latest release of snapbox (#13963) was geared at supporting cargo's needs in the hope that we can consolidate on testing frameworks.

Besides having a single set of semantics, benefits we'd gain include
- Updating of test snapshots
- Fancier redacting of test output (e.g. #13973)

This is the first incremental step in this direction.  This replaces direct assertions with snapbox assertions.  This still leaves all of the CLI output assertions. These will be done incrementally.

### How should we test and review this PR?

### Additional information
bors added a commit that referenced this pull request Jun 2, 2024
refactor: Transition direct assertions from cargo-test-support to snapbox

### What does this PR try to resolve?

Cargo has a bespoke testing framework for functional tests
- Extra stuff for us to maintain
- Don't leverage benefits from contributions related to other projects
- Less incentive to be thoroughly documented

UI tests are written using snapbox.  The latest release of snapbox (#13963) was geared at supporting cargo's needs in the hope that we can consolidate on testing frameworks.

Besides having a single set of semantics, benefits we'd gain include
- Updating of test snapshots
- Fancier redacting of test output (e.g. #13973)

This is the first incremental step in this direction.  This replaces direct assertions with snapbox assertions.  This still leaves all of the CLI output assertions. These will be done incrementally.

### How should we test and review this PR?

### Additional information
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 S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants