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

Add test for publish with [patch] + cleanup. #6544

Merged
merged 1 commit into from
Jan 12, 2019

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jan 12, 2019

This adds a test for publishing with a [patch] dependency. There is also a bunch of refactoring/cleanup to make that work better. Previously there were no tests for publishing with a dependency! A rough summary of the changes:

  • Remove all the duplicate code from support::publish, consolidate everything in support::registry.
  • Move API tokens into .cargo/credentials so it's easier to remove them in tests.
  • Rename registry/alt_registry to registry_url/alt_registry_url to be consistent with all the other function names and to avoid ambiguity.
  • Separate the default 'dl' and 'api' directories. Not really important, but is more consistent with the alt registry layout.

@rust-highfive
Copy link

r? @dwijnand

(rust_highfive has picked a reviewer for you, use r? to override)

@ehuss
Copy link
Contributor Author

ehuss commented Jan 12, 2019

@jethrogb The infrastructure for properly testing a [patch] upload wasn't really there, so this adds it. I'm not sure how familiar you are with Cargo's test infrastructure. There's a little bit of documentation at support/mod.rs and scattered throughout. Hopefully it's pretty self-explanatory, though.

Once this is ok'd, you can modify this test with something like the following to verify the new behavior in #6535.

diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs
index b6abb7ee..3c95fd74 100644
--- a/tests/testsuite/publish.rs
+++ b/tests/testsuite/publish.rs
@@ -829,17 +829,72 @@ fn publish_with_patch() {
             bar = { path = "bar" }
         "#,
         )
-        .file("src/main.rs", "extern crate bar; fn main() {}")
+        .file(
+            "src/main.rs",
+            "extern crate bar;
+             fn main() {
+                 bar::newfunc();
+             }",
+        )
         .file("bar/Cargo.toml", &basic_manifest("bar", "1.0.0"))
-        .file("bar/src/lib.rs", "")
+        .file("bar/src/lib.rs", "pub fn newfunc() {}")
         .build();

     // Check that it works with the patched crate.
     p.cargo("build").run();

+    // Check that verify fails with patched crate which has new functionality.
     p.cargo("publish --index")
         .arg(registry_url().to_string())
+        .with_stderr_contains("[..]newfunc[..]")
         .with_status(101)
-        .with_stderr_contains("[ERROR] published crates cannot contain [patch] sections")
         .run();
+
+    // Remove the usage of new functionality and try again.
+    p.change_file("src/main.rs", "extern crate bar; pub fn main() {}");
+
+    p.cargo("publish --index")
+        .arg(registry_url().to_string())
+        .with_stderr_contains("[WARNING] cargo will remove your [patch] section before publishing")
+        .run();
+
+    // Note, use of `registry` in the deps here is an artifact that this
+    // publishes to a fake, local registry that is pretending to be crates.io.
+    // Normal publishes would set it to null.
+    publish::validate_upload(
+        r#"
+        {
+          "authors": [],
+          "badges": {},
+          "categories": [],
+          "deps": [
+            {
+              "default_features": true,
+              "features": [],
+              "kind": "normal",
+              "name": "bar",
+              "optional": false,
+              "registry": "https://github.com/rust-lang/crates.io-index",
+              "target": null,
+              "version_req": "^1.0"
+            }
+          ],
+          "description": "foo",
+          "documentation": null,
+          "features": {},
+          "homepage": null,
+          "keywords": [],
+          "license": "MIT",
+          "license_file": null,
+          "links": null,
+          "name": "foo",
+          "readme": null,
+          "readme_file": null,
+          "repository": null,
+          "vers": "0.0.1"
+          }
+        "#,
+        "foo-0.0.1.crate",
+        &["Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
+    );
 }

@alexcrichton
Copy link
Member

Thanks @ehuss! r=me with a rebase

@ehuss ehuss force-pushed the test-publish-patch branch from e0dcb35 to b5144c7 Compare January 12, 2019 03:15
@ehuss
Copy link
Contributor Author

ehuss commented Jan 12, 2019

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jan 12, 2019

📌 Commit b5144c7 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jan 12, 2019

⌛ Testing commit b5144c7 with merge 2b4a5f1...

bors added a commit that referenced this pull request Jan 12, 2019
Add test for publish with [patch] + cleanup.

This adds a test for publishing with a `[patch]` dependency. There is also a bunch of refactoring/cleanup to make that work better. Previously there were no tests for publishing with a dependency! A rough summary of the changes:
- Remove all the duplicate code from `support::publish`, consolidate everything in `support::registry`.
- Move API tokens into `.cargo/credentials` so it's easier to remove them in tests.
- Rename `registry`/`alt_registry` to `registry_url`/`alt_registry_url` to be consistent with all the other function names and to avoid ambiguity.
- Separate the default 'dl' and 'api' directories. Not really important, but is more consistent with the alt registry layout.
@bors
Copy link
Contributor

bors commented Jan 12, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 2b4a5f1 to master...

@bors bors merged commit b5144c7 into rust-lang:master Jan 12, 2019
@dwijnand
Copy link
Member

Nice one!

bors added a commit to rust-lang/rust that referenced this pull request Jan 14, 2019
Update cargo

13 commits in 34320d212dca8cd27d06ce93c16c6151f46fcf2e..2b4a5f1f0bb6e13759e88ea9512527b0beba154f
2019-01-03 19:12:38 +0000 to 2019-01-12 04:13:12 +0000
- Add test for publish with [patch] + cleanup. (rust-lang/cargo#6544)
- Fix clippy warning (rust-lang/cargo#6546)
- Revert "Workaround by using yesterday's nightly" (rust-lang/cargo#6540)
- Adding feature-flags to `cargo publish` and `cargo package` (rust-lang/cargo#6453)
- Fix the Travis CI badge (rust-lang/cargo#6530)
- Add helpful text for Windows exceptions like Unix (rust-lang/cargo#6532)
- Report fix bugs to Rust instead of Cargo (rust-lang/cargo#6531)
- --{example,bin,bench,test} with no argument now lists all available targets (rust-lang/cargo#6505)
- Rebuild on mid build file modification (rust-lang/cargo#6484)
- Derive Clone for TomlDependency (rust-lang/cargo#6527)
- publish: rework the crates.io detection logic. (rust-lang/cargo#6525)
- avoid duplicates in ignore files (rust-lang/cargo#6521)
- Rustflags in metadata (rust-lang/cargo#6503)

r? @alexcrichton
@ehuss ehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants