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 some files to snapbox #14132

Merged
merged 4 commits into from
Jun 30, 2024
Merged

test: Migrate some files to snapbox #14132

merged 4 commits into from
Jun 30, 2024

Conversation

heisen-li
Copy link
Contributor

What does this PR try to resolve?

 tests/testsuite/credential_process.rs
 tests/testsuite/cross_compile.rs
 tests/testsuite/cross_publish.rs
 tests/testsuite/custom_target.rs

part of #14039

@rustbot
Copy link
Collaborator

rustbot commented Jun 24, 2024

r? @ehuss

rustbot has assigned @ehuss.
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 24, 2024
weihanglo
weihanglo reviewed Jun 24, 2024
View reviewed changes
tests/testsuite/credential_process.rs
[CREDENTIAL] [..]not_supported[..] get crates-io
{"v":1,"registry":{"index-url":"[..]","name":"crates-io","headers":[[..]"WWW-Authenticate: Cargo login_url=\"https://test-registry-login/me\""[..]]},"kind":"get","operation":"read"}
{"v":1,"registry":{"index-url":"[..]","name":"crates-io","headers":[[..]"WWW-Authenticate: Cargo login_url=/"https://test-registry-login/me/""[..]]},"kind":"get","operation":"read"}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need the leading slash for the value of login_url?
(And why it passed the CI 🤔)

Copy link
Contributor

Choose a reason for hiding this comment

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

That is likely escaping in json, rather than in Rust.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I'm not sure, but it will report an error if you don't add it. 😭

Copy link
Member

Choose a reason for hiding this comment

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

That is exotic to me. Maybe snapbox was trying to normalize backslash escaping?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless you do structured assertions with .json(), we don't know if \ was used for escaping or paths and convert it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, what can I do about this? There doesn't seem to be much, or am I misunderstanding?

Copy link
Contributor

Choose a reason for hiding this comment

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

For myself, I don't see something to be done with this. Its behaving as expected.

Copy link
Member

Choose a reason for hiding this comment

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

I am fine with us moving forward as we've identified the root cause.

@weihanglo weihanglo mentioned this pull request Jun 30, 2024
Closed
@weihanglo
Copy link
Member

Thank you.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 30, 2024

📌 Commit b1a157f 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 Jun 30, 2024
@bors
Copy link
Contributor

bors commented Jun 30, 2024

⌛ Testing commit b1a157f with merge 50e1e53...

@bors
Copy link
Contributor

bors commented Jun 30, 2024

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

@bors bors merged commit 50e1e53 into rust-lang:master Jun 30, 2024
22 checks passed
@heisen-li heisen-li deleted the cre_cro_cus branch July 1, 2024 01:21
@weihanglo weihanglo mentioned this pull request Jul 3, 2024
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
ehuss added a commit to ehuss/cargo that referenced this pull request Jan 26, 2025
@ehuss
Update tests to deal with linker warnings
c35cb56 
Nightly recently introduced the `linker-messages` lint which prints any
messages from linkers. These tests were triggering that lint because
they were passing missing directories to the linker. This fixes it by
creating empty directories to pass to the linker.

Note that this lint will be downgraded soon via
rust-lang/rust#136098, but it seemed worthwhile
to fix the underlying problem.

This also fixes a problem where build_script_needed_for_host_and_target
was not testing for the correct command-line flags. These got lost
in rust-lang#14132.
@ehuss ehuss mentioned this pull request Jan 26, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jan 28, 2025
@ehuss
Update tests to fix nightly errors (#15110)
fd4f492 
There were some changes in the latest nightly which is breaking some
tests:

* rust-lang/rust#133154 updated the wording of a
message. I adjusted the test to be less sensitive to the exact wording.
* rust-lang/rust#119286 added the
`linker-messages` lint which shows the output from the linker. Some of
our tests were passing dummy flags to the linker, which it was
complaining about. This updates it so that it uses real directories.
* This also fixes an incidental issue, where
`build_script_needed_for_host_and_target` was not testing for the
correct command-line flags. These got lost in
#14132.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epage epage epage left review comments

@weihanglo weihanglo weihanglo approved these changes

Assignees

@ehuss ehuss

Labels
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.
6 participants
@heisen-li @rustbot @weihanglo @bors @epage @ehuss