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

Fix build #1816

Merged
merged 2 commits into from
Aug 9, 2018
Merged

Fix build #1816

merged 2 commits into from
Aug 9, 2018

Conversation

weiznich
Copy link
Member

@weiznich weiznich commented Aug 8, 2018

Fix the CI

(I don't know why it broke without any warning. Those errors are caused by this commit)

@weiznich weiznich requested a review from a team August 8, 2018 13:22
@@ -9,6 +9,7 @@ documentation = "https://diesel.rs/guides/getting-started"
homepage = "https://diesel.rs"
repository = "https://github.com/diesel-rs/diesel"
keywords = ["diesel", "migrations", "cli"]
autotests = false
Copy link
Member

Choose a reason for hiding this comment

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

Does this skip running unit tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not change anything on the number of executed unit tests.

Help statement from cargo:

warning: An explicit [[test]] section is specified in Cargo.toml which currently
disables Cargo from automatically inferring other test targets.
This inference behavior will change in the Rust 2018 edition and the following
files will be included as a test target:

  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/helpers.rs
  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/as_changeset.rs
  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/associations.rs
  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/queryable.rs
  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/identifiable.rs
  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/schema.rs
  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/queryable_by_name.rs
  • /home/weiznich/Dokumente/rust/diesel/diesel_derives/tests/insertable.rs
    This is likely to break cargo build or cargo test as these files may not be
    ready to be compiled as a test target today. You can future-proof yourself
    and disable this warning by adding autotests = false to your [package]
    section. You may also move the files to a location where Cargo would not
    automatically infer them to be a target, such as in subfolders.
    For more information on this warning you can consult
    Make explicit opt-out of target discovery neccessary for Rust 2018 rust-lang/cargo#5330

(The same applies for diesel_derive)

Copy link
Member

Choose a reason for hiding this comment

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

Those are the integration tests. Are the tests in src still run (if there are any)?

Copy link
Member

Choose a reason for hiding this comment

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

Also I don't think we need to handle warnings for the 2018 edition just yet -- I'm pretty sure we actually want to fix this the other way, and remove the [[test]] section (not 100% sure yet, waiting until 2018 is in beta to figure it out)

Copy link
Member Author

Choose a reason for hiding this comment

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

See the linked Cargo issue. Setting autotest = false is basically just the current behaviour. This only disables the detection of integration tests in tests/. For this this means that we have only on entry point into our integration tests as is it currently defined in our Cargo.toml
I do not see any downside of doing this now, this will just silence the warnings by explicitly opting in to the current default behaviour.

Copy link
Member

Choose a reason for hiding this comment

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

Ok sounds good

@@ -7,6 +7,7 @@ description = "You should not use this crate directly, it is internal to Diesel.
documentation = "https://diesel.rs/guides/"
homepage = "https://diesel.rs"
repository = "https://github.com/diesel-rs/diesel/tree/master/diesel_derives"
autotests = false
Copy link
Member

Choose a reason for hiding this comment

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

Does this skip running unit tests?

@weiznich weiznich merged commit 14a5557 into diesel-rs:master Aug 9, 2018
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.

2 participants