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.

Sign up for GitHub

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

Jump to bottom

test: migrate package* and publish* to snapbox #14130

Merged
merged 5 commits into from
Jun 27, 2024
Merged

test: migrate package* and publish* to snapbox #14130

merged 5 commits into from
Jun 27, 2024

Conversation

eth3lbert
Copy link
Contributor

What does this PR try to resolve?

Part of #14039.

Migrate following to snapbox:

  • tests/testsuite/package.rs
  • tests/testsuite/package_features.rs
  • tests/testsuite/publish.rs
  • tests/testsuite/publish_lockfile.rs

@rustbot
Copy link
Collaborator

rustbot commented Jun 23, 2024

r? @epage

rustbot has assigned @epage.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 23, 2024
@eth3lbert eth3lbert force-pushed the snapbox-package-n-publish branch 4 times, most recently from a93d718 to 57c83d8 Compare June 23, 2024 23:42
eth3lbert
eth3lbert commented Jun 24, 2024
View reviewed changes
tests/testsuite/package.rs
Comment on lines +2403 to +2509
Caused by:
[NOT_FOUND]
Copy link
Contributor Author

@eth3lbert eth3lbert Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

A new [NOT_FOUND] pattern in windows:
https://github.com/rust-lang/cargo/actions/runs/9637273398/job/26576234294?pr=14130#step:11:4039

    8      -   [NOT_FOUND]
        8 +   Access is denied. (os error 5)

Since the error message is Access is denied. I'm not sure if we still want it to be displayed as [NOT_FOUND]. Alternatively, we could handle this with a platform-specific assertion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Go ahead and redact it? We can re-evaluate as we go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Redacted!

eth3lbert
eth3lbert commented Jun 24, 2024
View reviewed changes
tests/testsuite/package.rs
.run();
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with_stderr_does_not_contain is being used in this function.

eth3lbert
eth3lbert commented Jun 24, 2024
View reviewed changes
tests/testsuite/package_features.rs
.run();
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with_stderr_does_not_contain is being used in this function.

eth3lbert
eth3lbert commented Jun 24, 2024
View reviewed changes
tests/testsuite/publish.rs
@@ -1353,6 +1335,7 @@ You may press ctrl-c [..]
);
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with_stderr_does_not_contain is being used in this function.

eth3lbert
eth3lbert commented Jun 24, 2024
View reviewed changes
tests/testsuite/publish.rs
[UPDATING] `alternative` index
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
[PACKAGED] 3 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
[UPLOADING] foo v0.0.1 ([ROOT]/foo)
[ERROR] failed to publish to registry at http://127.0.0.1:[..]/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[ERROR] failed to publish to registry at http://127.0.0.1:64705/

Could consider auto-redact the port to [REGISTRY_PORT].

@bors
Copy link
Contributor

bors commented Jun 24, 2024

☔ The latest upstream changes (presumably #13960) made this pull request unmergeable. Please resolve the merge conflicts.

linyihai
linyihai reviewed Jun 25, 2024
View reviewed changes
tests/testsuite/package.rs
"\
{}
p.cargo("package --list").with_stdout_data(str![[r#"
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
Copy link
Contributor

Choose a reason for hiding this comment

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

this is some magic number, could it be ignored?

not a block, just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this eliminates the need for format! and it can be automatically generated by snapshot=overwrite, I believe this approach is likely acceptable.

Please let me know if this is not appropriate. If so, I'll revert to using format! again.

epage
epage reviewed Jun 26, 2024
View reviewed changes
tests/testsuite/package.rs Outdated Show resolved Hide resolved
@eth3lbert eth3lbert force-pushed the snapbox-package-n-publish branch 4 times, most recently from e6cb871 to c6f2fc8 Compare June 27, 2024 04:54
@rustbot rustbot added the A-testing-cargo-itself Area: cargo's tests label Jun 27, 2024
@epage
Copy link
Contributor

epage commented Jun 27, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jun 27, 2024

📌 Commit a2b706b has been approved by epage

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 27, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 27, 2024
@bors
Copy link
Contributor

bors commented Jun 27, 2024

⌛ Testing commit a2b706b with merge 0664c7e...

@bors
Copy link
Contributor

bors commented Jun 27, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing 0664c7e to master...

@bors bors merged commit 0664c7e into rust-lang:master Jun 27, 2024
22 checks passed
@bors bors mentioned this pull request Jun 27, 2024
Merged
@eth3lbert eth3lbert deleted the snapbox-package-n-publish branch June 27, 2024 18:55
This was referenced Jun 29, 2024
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 3, 2024
@bors
Auto merge of rust-lang#127258 - weihanglo:update-cargo, r=weihanglo
d68fe4e 
Update cargo

23 commits in 4ed7bee47f7dd4416b36fada1909e9a62c546246..a515d463427b3912ec0365d106791f88c1c14e1b
2024-06-25 16:28:22 +0000 to 2024-07-02 20:53:36 +0000
- test: migrate rust_version and rustc* to snapbox (rust-lang/cargo#14177)
- test: mirgate fix* and future_incompat_report to snapbox (rust-lang/cargo#14173)
- test：migrate `edition/error` to snapbox (rust-lang/cargo#14175)
- chore(deps): update compatible (rust-lang/cargo#14174)
- refactor(source): Clean up after PathSource/RecursivePathSource split (rust-lang/cargo#14169)
- test: Migrate some files to snapbox (rust-lang/cargo#14132)
- test:  fix several assertions (rust-lang/cargo#14167)
- test: replace glob with explicit unordered calls (rust-lang/cargo#14166)
- Make it clear that `CARGO_CFG_TARGET_FAMILY` is multi-valued (rust-lang/cargo#14165)
- Document `CARGO_CFG_TARGET_ABI` (rust-lang/cargo#14164)
- test: Migrate git to snapbox (rust-lang/cargo#14159)
- test: migrate some files to snapbox (rust-lang/cargo#14158)
- test: migrate registry and registry_auth to snapbox (rust-lang/cargo#14149)
- gix: remove `revision` feature from cargo (rust-lang/cargo#14160)
- test: migrate package* and publish* to snapbox (rust-lang/cargo#14130)
- More `update --breaking` tests (rust-lang/cargo#14049)
- test: migrate clean to snapbox (rust-lang/cargo#14096)
- Allow `unexpected_builtin_cfgs` lint in `user_specific_cfgs` test (rust-lang/cargo#14153)
- test: migrate search, source_replacement and standard_lib to snapbox (rust-lang/cargo#14151)
- Docs: Update config summary to include missing keys. (rust-lang/cargo#14145)
- test: migrate `dep_info/diagnostics/direct_minimal_versions` to snapbox (rust-lang/cargo#14143)
- Docs: Remove duplicate `strip` section. (rust-lang/cargo#14146)
- Docs: Fix curly quotes in config docs. (rust-lang/cargo#14144)
@rustbot rustbot added this to the 1.81.0 milestone Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linyihai linyihai linyihai left review comments

@epage epage epage left review comments

Assignees

@epage epage

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
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.
5 participants
@eth3lbert @rustbot @bors @epage @linyihai