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(priv_dep): add test for verify public is respected recursively #13183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

test(priv_dep): add test for verify public is respected recursively #13183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions crates/cargo-test-support/src/registry.rs
View file
Open in desktop
Expand Up @@ -558,6 +558,7 @@ pub struct Dependency {
package: Option<String>,
optional: bool,
default_features: bool,
public: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to break this out into its own PR for us to merge immiedately

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

}

/// Entry with data that corresponds to [`tar::EntryType`].
Expand Down Expand Up @@ -1428,6 +1429,7 @@ impl Package {
"kind": dep.kind,
"registry": registry_url,
"package": dep.package,
"public": dep.public
})
})
.collect::<Vec<_>>();
Expand Down Expand Up @@ -1678,6 +1680,7 @@ impl Dependency {
optional: false,
registry: None,
default_features: true,
public: false,
}
}

Expand Down Expand Up @@ -1731,6 +1734,12 @@ impl Dependency {
self
}

/// Changes this to an public dependency.
pub fn public(&mut self, public: bool) -> &mut Self {
self.public = public;
self
}

/// Adds `default-features = false` if the argument is `false`.
pub fn default_features(&mut self, default_features: bool) -> &mut Self {
self.default_features = default_features;
Expand Down
219 changes: 218 additions & 1 deletion tests/testsuite/pub_priv.rs
View file
Open in desktop
@@ -1,7 +1,7 @@
//! Tests for public/private dependencies.

use cargo_test_support::project;
use cargo_test_support::registry::Package;
use cargo_test_support::registry::{Dependency, Package};

#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")]
fn exported_priv_warning() {
Expand Down Expand Up @@ -479,3 +479,220 @@ fn allow_priv_in_custom_build() {
)
.run()
}

// A indirectly add D as private dependency.
Copy link
Contributor

Choose a reason for hiding this comment

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

These tests don't make it too clear that they are not expected behavior but that some help show existing bugs. We should call that out, linking to the issue in question

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been unsure whether these should be merged before the related fix. Thinking on it more, and with rust-lang/rust#119428 being moved to the rust repo, I suspect we should hold off on tests that demonstrate bad behavior. The problem is that the fix would be merged into the rust repo but they can't update the tests in the same commit, adding more frustration to the process of fixing the bugs.

Sorry that I hadn't thought of it before you had put more time into polishing this up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is ok to close this PR or add WIP to the title until the issue fixed in rust repo.

// A -> B -> C -> D
#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")]
fn recursive_package_pub_no_warning() {
Package::new("grandparent_bar", "0.1.0")
.file("src/lib.rs", "pub struct FromPub;")
.publish();
Package::new("parent_bar", "0.1.0")
.cargo_feature("public-dependency")
.add_dep(Dependency::new("grandparent_bar", "0.1.0").public(true))
.file(
"src/lib.rs",
"
extern crate grandparent_bar;
pub use grandparent_bar::*;
",
)
.publish();
Package::new("pub_dep", "0.1.0")
.cargo_feature("public-dependency")
.add_dep(Dependency::new("parent_bar", "0.1.0").public(true))
.file(
"src/lib.rs",
"
extern crate parent_bar;
pub use parent_bar::*;
",
)
.publish();
let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "foo"
version = "0.0.1"

[dependencies]
pub_dep = {version = "0.1.0", public = true}
"#,
)
.file(
"src/lib.rs",
"
extern crate pub_dep;
pub fn use_pub(_: pub_dep::FromPub) {}
",
)
.build();

p.cargo("check --message-format=short")
.masquerade_as_nightly_cargo(&["public-dependency"])
.with_stderr(
"\
[UPDATING] `[..]` index
[DOWNLOADING] crates ...
[DOWNLOADED] pub_dep v0.1.0 ([..])
[DOWNLOADED] parent_bar v0.1.0 ([..])
[DOWNLOADED] grandparent_bar v0.1.0 ([..])
[CHECKING] grandparent_bar v0.1.0
[CHECKING] parent_bar v0.1.0
[CHECKING] pub_dep v0.1.0
[CHECKING] foo v0.0.1 ([..])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run()
}

// A indirectly add D as private dependency.
// A -> B -> C -> D
#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")]
fn recursive_package_priv_warning() {
Package::new("grandparent_bar", "0.1.0")
.file("src/lib.rs", "pub struct FromPriv;")
.publish();
Package::new("parent_bar", "0.1.0")
.cargo_feature("public-dependency")
.add_dep(Dependency::new("grandparent_bar", "0.1.0").public(true))
.file(
"src/lib.rs",
"
extern crate grandparent_bar;
pub use grandparent_bar::*;
",
)
.publish();
Package::new("priv_dep", "0.1.0")
.cargo_feature("public-dependency")
.add_dep(Dependency::new("parent_bar", "0.1.0").public(true))
.file(
"src/lib.rs",
"
extern crate parent_bar;
pub use parent_bar::*;
",
)
.publish();
let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "foo"
version = "0.0.1"

[dependencies]
priv_dep = "0.1.0"
"#,
)
.file(
"src/lib.rs",
"
extern crate priv_dep;
pub fn use_pub(_: priv_dep::FromPriv) {}
",
)
.build();

p.cargo("check --message-format=short")
.masquerade_as_nightly_cargo(&["public-dependency"])
.with_stderr(
"\
[UPDATING] `[..]` index
[DOWNLOADING] crates ...
[DOWNLOADED] priv_dep v0.1.0 ([..])
[DOWNLOADED] parent_bar v0.1.0 ([..])
[DOWNLOADED] grandparent_bar v0.1.0 ([..])
[CHECKING] grandparent_bar v0.1.0
[CHECKING] parent_bar v0.1.0
[CHECKING] priv_dep v0.1.0
[CHECKING] foo v0.0.1 ([..])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run()
}

// A indirectly add D as public dependency, then directly add D as private dependency.
// A -> B -> C -> D
// \ _ _ _ _ _ /|\
#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")]
fn recursive_package_pub_priv_together() {
Package::new("grandparent_bar", "0.1.0")
.file("src/lib.rs", "pub struct FromPriv;")
.publish();
Package::new("parent_bar", "0.1.0")
.cargo_feature("public-dependency")
.add_dep(Dependency::new("grandparent_bar", "0.1.0").public(true))
.file(
"src/lib.rs",
"
extern crate grandparent_bar;
pub use grandparent_bar::*;
",
)
.publish();
Package::new("pub_dep", "0.1.0")
.cargo_feature("public-dependency")
.add_dep(Dependency::new("parent_bar", "0.1.0").public(true))
.file(
"src/lib.rs",
"
extern crate parent_bar;
pub use parent_bar::*;
",
)
.publish();
let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["public-dependency"]

[package]
name = "foo"
version = "0.0.1"

[dependencies]
pub_dep = {version = "0.1.0", public = true}
grandparent_bar = "0.1.0"
"#,
)
.file(
"src/lib.rs",
"
extern crate pub_dep;
extern crate grandparent_bar;

pub fn use_pub(_: grandparent_bar::FromPriv) {}
",
)
.build();

p.cargo("check --message-format=short")
.masquerade_as_nightly_cargo(&["public-dependency"])
.with_stderr(
"\
[UPDATING] `[..]` index
[DOWNLOADING] crates ...
[DOWNLOADED] pub_dep v0.1.0 ([..])
[DOWNLOADED] parent_bar v0.1.0 ([..])
[DOWNLOADED] grandparent_bar v0.1.0 ([..])
[CHECKING] grandparent_bar v0.1.0
[CHECKING] parent_bar v0.1.0
[CHECKING] pub_dep v0.1.0
[CHECKING] foo v0.0.1 ([..])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run()
}