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

Support running on stable #4

Closed
idubrov opened this issue May 8, 2019 · 14 comments · Fixed by #8
Closed

Support running on stable #4

idubrov opened this issue May 8, 2019 · 14 comments · Fixed by #8

Comments

@idubrov
Copy link
Contributor

idubrov commented May 8, 2019

It's possible to make it run on stable by using harness = false in Cargo.toml, with the following drawbacks:

  1. Extra configuration will be required in Cargo.toml.
  2. Special macro will be used to provide main function. Note that this function will have to be named main due to Tracking issue for main feature rust-lang/rust#29634 being unstable.
  3. In case of binary target (bin/* or main.rs), extra attribute will be needed on a real main function to disable it when running with tests.
  4. Some magic will be needed for registering tests (could use https://crates.io/crates/ctor or https://crates.io/crates/inventory).

Perhaps, can use https://crates.io/crates/libtest crate (which, I think, was pulled from rust itself) to run the tests.

@idubrov
Copy link
Contributor Author

idubrov commented May 8, 2019

So, the ergonomics will be:

  1. Use harness = false in Cargo.toml
  2. Put #![datatest::test_harness] attribute at the top of each test / binary.

@sunshowers
Copy link

Hi @idubrov! Was curious what the status is -- are you working on this at the moment? If not then I or one of my colleagues might jump on this one. We really desire to use datatest on stable Rust :)

@idubrov
Copy link
Contributor Author

idubrov commented Jul 19, 2019

No, not really.

I started doing some experiments, but found that getting exactly the same behavior does not seem to be possible in stable, so I abandoned my experiments. Since we are currently very much tied to nightly anyways, there was no pressure for that 😅

Looking at my notes above, I'm not sure they would work.

  1. I think, one issue with harness = false was that all regular #[test] would be removed by the compiler, which is very unfortunate as I would like to mix regular tests and #[datatest(..)] tests. I guess, there could be datatest::test attribute instead? But then it is not clear how to "register" all these tests (it seems like compiler generates some plumbing to accumulate all of these tests into a Vec of TestDescAndFn
  2. ctor/inventory might also not work due to inventory::submit!() doesn't work unless code from downstream crate is actually used dtolnay/inventory#7

The alternative would be to abandon current ergonomics and simply generate one single test which would loop over set of tests internally. However, that would be a pretty big deal for me -- the main advantage currently is that each test case behaves like a separate #[test] from the point of view of cargo test / IntelliJ plugins.

Though, I guess it could be done based on Rust detection nightly vs stable.

I was also hoping for some progress on rust-lang/rust#50297, but it doesn't move anywhere it seems.

@sunshowers
Copy link

Thanks for the detailed notes!

In our case 1 is not a huge issue as our datatests are not mixed with regular ones but I totally see what you mean. Curious, would you be ok with that limitation on stable only?

2 is really unfortunate :(

Yeah, I agree that having a separate test per data file is essential.

@idubrov
Copy link
Contributor Author

idubrov commented Jul 19, 2019

Curious, would you be ok with that limitation on stable only?

Yeah, sure.

idubrov pushed a commit that referenced this issue Aug 17, 2019
Started working on #4 (support for stable Rust). First issue we need to
solve is to get access to the harness (since we don't really want to
implement it ourselves).

There is https://crates.io/crates/libtest crate, which is recent version
of Rust internal test harness, extracted as a crate. However, it only
compiles on nightly, so it won't help us here.

There is also https://crates.io/crates/rustc-test, but it is 2 years
old. I haven't checked its features, but might not support some of the
desired functionality (like, JSON output in tests? colored output?).

So, the third option (which I'm using here) is to use `test` crate from
the Rust itself and also set `RUSTC_BOOTSTRAP=1` for our crate so we
can access it on stable channel. Not great, but works for now.

Second issue is to get access to the tests. On nightly, we use
`#[test_case]` to hijack Rust tests registration so we can get access to
them in nightly.

Cannot do that on stable. What would help here is something along the
lines of https://internals.rust-lang.org/t/idea-global-static-variables-extendable-at-compile-time/9879
or https://internals.rust-lang.org/t/pre-rfc-add-language-support-for-global-constructor-functions.
Don't have that, so we use https://crates.io/crates/ctor crate to build
our own registry of tests, similar to https://crates.io/crates/inventory.

The caveat here is potentially hitting dtolnay/inventory#7
issue which would manifest itself as test being silently ignored. Not
great, but let's see how bad it will be.

Third piece of the puzzle is to intercept execution of tests. This is
done by asking users to use `harness = false` in their `Cargo.toml`, in
which case we take full control of test execution.

Finally, the last challenge is that with `harness = false`, we don't
have a good way to intercept "standard" tests (`#[test]`): https://users.rust-lang.org/t/capturing-test-when-harness-false-in-cargo-toml/28115

So, the plan here is to provide `#[datatest::test]` attribute that will
behave similar to built-in `#[test]` attribute, but will use our own
registry for tests. No need to support `#[bench]` as it is not supported
on stable channel anyway.

The caveat in this case is that if you use built-in `#[test]`, your test
will be silently ignored. Not great, not sure what to do about it.

Proper solution, of course, would be driving RFC for custom test
frameworks: rust-lang/rust#50297 😅
idubrov pushed a commit that referenced this issue Aug 17, 2019
Started working on #4 (support for stable Rust). First issue we need to
solve is to get access to the harness (since we don't really want to
implement it ourselves).

There is https://crates.io/crates/libtest crate, which is recent version
of Rust internal test harness, extracted as a crate. However, it only
compiles on nightly, so it won't help us here.

There is also https://crates.io/crates/rustc-test, but it is 2 years
old. I haven't checked its features, but might not support some of the
desired functionality (like, JSON output in tests? colored output?).

So, the third option (which I'm using here) is to use `test` crate from
the Rust itself and also set `RUSTC_BOOTSTRAP=1` for our crate so we
can access it on stable channel. Not great, but works for now.

Second issue is to get access to the tests. On nightly, we use
`#[test_case]` to hijack Rust tests registration so we can get access to
them in nightly.

Cannot do that on stable. What would help here is something along the
lines of https://internals.rust-lang.org/t/idea-global-static-variables-extendable-at-compile-time/9879
or https://internals.rust-lang.org/t/pre-rfc-add-language-support-for-global-constructor-functions.
Don't have that, so we use https://crates.io/crates/ctor crate to build
our own registry of tests, similar to https://crates.io/crates/inventory.

The caveat here is potentially hitting dtolnay/inventory#7
issue which would manifest itself as test being silently ignored. Not
great, but let's see how bad it will be.

Third piece of the puzzle is to intercept execution of tests. This is
done by asking users to use `harness = false` in their `Cargo.toml`, in
which case we take full control of test execution.

Finally, the last challenge is that with `harness = false`, we don't
have a good way to intercept "standard" tests (`#[test]`): https://users.rust-lang.org/t/capturing-test-when-harness-false-in-cargo-toml/28115

So, the plan here is to provide `#[datatest::test]` attribute that will
behave similar to built-in `#[test]` attribute, but will use our own
registry for tests. No need to support `#[bench]` as it is not supported
on stable channel anyway.

The caveat in this case is that if you use built-in `#[test]`, your test
will be silently ignored. Not great, not sure what to do about it.

Proper solution, of course, would be driving RFC for custom test
frameworks: rust-lang/rust#50297 😅
idubrov pushed a commit that referenced this issue Aug 17, 2019
Started working on #4 (support for stable Rust). First issue we need to
solve is to get access to the harness (since we don't really want to
implement it ourselves).

There is https://crates.io/crates/libtest crate, which is recent version
of Rust internal test harness, extracted as a crate. However, it only
compiles on nightly, so it won't help us here.

There is also https://crates.io/crates/rustc-test, but it is 2 years
old. I haven't checked its features, but might not support some of the
desired functionality (like, JSON output in tests? colored output?).

So, the third option (which I'm using here) is to use `test` crate from
the Rust itself and also set `RUSTC_BOOTSTRAP=1` for our crate so we
can access it on stable channel. Not great, but works for now.

Second issue is to get access to the tests. On nightly, we use
`#[test_case]` to hijack Rust tests registration so we can get access to
them in nightly.

Cannot do that on stable. What would help here is something along the
lines of https://internals.rust-lang.org/t/idea-global-static-variables-extendable-at-compile-time/9879
or https://internals.rust-lang.org/t/pre-rfc-add-language-support-for-global-constructor-functions.
Don't have that, so we use https://crates.io/crates/ctor crate to build
our own registry of tests, similar to https://crates.io/crates/inventory.

The caveat here is potentially hitting dtolnay/inventory#7
issue which would manifest itself as test being silently ignored. Not
great, but let's see how bad it will be.

Third piece of the puzzle is to intercept execution of tests. This is
done by asking users to use `harness = false` in their `Cargo.toml`, in
which case we take full control of test execution.

Finally, the last challenge is that with `harness = false`, we don't
have a good way to intercept "standard" tests (`#[test]`): https://users.rust-lang.org/t/capturing-test-when-harness-false-in-cargo-toml/28115

So, the plan here is to provide `#[datatest::test]` attribute that will
behave similar to built-in `#[test]` attribute, but will use our own
registry for tests. No need to support `#[bench]` as it is not supported
on stable channel anyway.

The caveat in this case is that if you use built-in `#[test]`, your test
will be silently ignored. Not great, not sure what to do about it.

Proper solution, of course, would be driving RFC for custom test
frameworks: rust-lang/rust#50297 😅
idubrov pushed a commit that referenced this issue Aug 17, 2019
Started working on #4 (support for stable Rust). First issue we need to solve is to get access to the harness (since we don't really want to implement it ourselves).

There is https://crates.io/crates/libtest crate, which is recent version of Rust internal test harness, extracted as a crate. However, it only compiles on nightly, so it won't help us here.

There is also https://crates.io/crates/rustc-test, but it is 2 years old. I haven't checked its features, but might not support some of the desired functionality (like, JSON output in tests? colored output?).

So, the third option (which I'm using here) is to use `test` crate from the Rust itself and also set `RUSTC_BOOTSTRAP=1` for our crate so we can access it on stable channel. Not great, but works for now.

Second issue is to get access to the tests. On nightly, we use `#[test_case]` to hijack Rust tests registration so we can get access to them in nightly.

Cannot do that on stable. What would help here is something along the lines of https://internals.rust-lang.org/t/idea-global-static-variables-extendable-at-compile-time/9879 or https://internals.rust-lang.org/t/pre-rfc-add-language-support-for-global-constructor-functions. Don't have that, so we use https://crates.io/crates/ctor crate to build our own registry of tests, similar to https://crates.io/crates/inventory.

The caveat here is potentially hitting dtolnay/inventory#7 issue which would manifest itself as test being silently ignored. Not great, but let's see how bad it will be.

Third piece of the puzzle is to intercept execution of tests. This is done by asking users to use `harness = false` in their `Cargo.toml`, in which case we take full control of test execution.

Finally, the last challenge is that with `harness = false`, we don't have a good way to intercept "standard" tests (`#[test]`): https://users.rust-lang.org/t/capturing-test-when-harness-false-in-cargo-toml/28115

So, the plan here is to provide `#[datatest::test]` attribute that will behave similar to built-in `#[test]` attribute, but will use our own registry for tests. No need to support `#[bench]` as it is not supported on stable channel anyway.

The caveat in this case is that if you use built-in `#[test]`, your test will be silently ignored. Not great, not sure what to do about it.

Proper solution, of course, would be driving RFC for custom test frameworks: rust-lang/rust#50297 😅

Partially fixes #4 (still missing support for standard tests and also documentation).
idubrov pushed a commit that referenced this issue Aug 17, 2019
Started working on #4 (support for stable Rust). First issue we need to solve is to get access to the harness (since we don't really want to implement it ourselves).

There is https://crates.io/crates/libtest crate, which is recent version of Rust internal test harness, extracted as a crate. However, it only compiles on nightly, so it won't help us here.

There is also https://crates.io/crates/rustc-test, but it is 2 years old. I haven't checked its features, but might not support some of the desired functionality (like, JSON output in tests? colored output?).

So, the third option (which I'm using here) is to use `test` crate from the Rust itself and also set `RUSTC_BOOTSTRAP=1` for our crate so we can access it on stable channel. Not great, but works for now.

Second issue is to get access to the tests. On nightly, we use `#[test_case]` to hijack Rust tests registration so we can get access to them in nightly.

Cannot do that on stable. What would help here is something along the lines of https://internals.rust-lang.org/t/idea-global-static-variables-extendable-at-compile-time/9879 or https://internals.rust-lang.org/t/pre-rfc-add-language-support-for-global-constructor-functions. Don't have that, so we use https://crates.io/crates/ctor crate to build our own registry of tests, similar to https://crates.io/crates/inventory.

The caveat here is potentially hitting dtolnay/inventory#7 issue which would manifest itself as test being silently ignored. Not great, but let's see how bad it will be.

Third piece of the puzzle is to intercept execution of tests. This is done by asking users to use `harness = false` in their `Cargo.toml`, in which case we take full control of test execution.

Finally, the last challenge is that with `harness = false`, we don't have a good way to intercept "standard" tests (`#[test]`): https://users.rust-lang.org/t/capturing-test-when-harness-false-in-cargo-toml/28115

So, the plan here is to provide `#[datatest::test]` attribute that will behave similar to built-in `#[test]` attribute, but will use our own registry for tests. No need to support `#[bench]` as it is not supported on stable channel anyway.

The caveat in this case is that if you use built-in `#[test]`, your test will be silently ignored. Not great, not sure what to do about it.

Proper solution, of course, would be driving RFC for custom test frameworks: rust-lang/rust#50297 😅

Partially fixes #4 (still missing support for standard tests and also documentation).
@idubrov
Copy link
Contributor Author

idubrov commented Aug 17, 2019

Okay, I've done my first stab at supporting stable: #8. Still does not support standard tests, but otherwise seems like it works (with few caveats)?

idubrov pushed a commit that referenced this issue Aug 18, 2019
Started working on #4 (support for stable Rust). First issue we need to solve is to get access to the harness (since we don't really want to implement it ourselves).

There is https://crates.io/crates/libtest crate, which is recent version of Rust internal test harness, extracted as a crate. However, it only compiles on nightly, so it won't help us here.

There is also https://crates.io/crates/rustc-test, but it is 2 years old. I haven't checked its features, but might not support some of the desired functionality (like, JSON output in tests? colored output?).

So, the third option (which I'm using here) is to use `test` crate from the Rust itself and also set `RUSTC_BOOTSTRAP=1` for our crate so we can access it on stable channel. Not great, but works for now.

Second issue is to get access to the tests. On nightly, we use `#[test_case]` to hijack Rust tests registration so we can get access to them in nightly.

Cannot do that on stable. What would help here is something along the lines of https://internals.rust-lang.org/t/idea-global-static-variables-extendable-at-compile-time/9879 or https://internals.rust-lang.org/t/pre-rfc-add-language-support-for-global-constructor-functions. Don't have that, so we use https://crates.io/crates/ctor crate to build our own registry of tests, similar to https://crates.io/crates/inventory.

The caveat here is potentially hitting dtolnay/inventory#7 issue which would manifest itself as test being silently ignored. Not great, but let's see how bad it will be.

Third piece of the puzzle is to intercept execution of tests. This is done by asking users to use `harness = false` in their `Cargo.toml`, in which case we take full control of test execution.

Finally, the last challenge is that with `harness = false`, we don't have a good way to intercept "standard" tests (`#[test]`): https://users.rust-lang.org/t/capturing-test-when-harness-false-in-cargo-toml/28115

So, the plan here is to provide `#[datatest::test]` attribute that will behave similar to built-in `#[test]` attribute, but will use our own registry for tests. No need to support `#[bench]` as it is not supported on stable channel anyway.

The caveat in this case is that if you use built-in `#[test]`, your test will be silently ignored. Not great, not sure what to do about it.

Proper solution, of course, would be driving RFC for custom test frameworks: rust-lang/rust#50297 😅

Partially fixes #4 (still missing support for standard tests and also documentation).
@idubrov
Copy link
Contributor Author

idubrov commented Aug 18, 2019

I would call it done-ish, seems to work on stable now. Differences from nightly usage:

  1. Set harness = false for your test. If you don't, all tests will be silently ignored 🙀, maybe, I can use #[ctor::dtor] to check if test runner was actually called.
  2. Use datatest::harness!(); in the test file. Otherwise, it will fail to run.
  3. Add use datatest::test; everywhere you use regular #[test]. If you don't, regular tests will be silently ignored 🥶. Not sure if I can add any kind of protection here, with harness = false rustc simply discards all the #[test], it seems... 🤔

@sunshowers
Copy link

Thank you for your work!

I tried using it and ran into the following issue. It's possible this got masked for you when you set RUSTC_BOOTSTRAP in build.rs -- it might make sense to extract the datatest_stable test into another crate to be sure.

error[E0658]: use of unstable library feature 'custom_test_frameworks': custom test frameworks are an unstable feature
  --> testsuite/libra_fuzzer/tests/artifacts.rs:19:1
   |
19 | / #[datatest::files("artifacts", {
20 | |     artifact_path in r"^.*/.*"
21 | | })]
   | |___^
   |
   = note: for more information, see https://github.com/rust-lang/rust/issues/50297
   = help: add #![feature(custom_test_frameworks)] to the crate attributes to ena
ble

@idubrov
Copy link
Contributor Author

idubrov commented Aug 19, 2019

I think, this happens if you run it on nightly BUT don't enable #![feature(custom_test_frameworks)]. Currently it tries to detect which version it runs on by using version_check (https://github.com/commure/datatest/blob/master/build.rs#L4-L9) and then it either uses nightly-only #[test_case] or uses ctor-based registry for stable.

It's a bit of a mess right now :(

@sunshowers
Copy link

Ahh I see, yeah that's exactly what I was trying to do! OK, good to know that it'll work better on stable Rust, I'll give that a shot.

BTW there has been some noise about forbidding RUSTC_BOOTSTRAP in build scripts uploaded to crates.io: rust-lang/cargo#6608

@nox
Copy link

nox commented Aug 23, 2019

BTW there has been some noise about forbidding RUSTC_BOOTSTRAP in build scripts uploaded to crates.io: rust-lang/cargo#6608

Please don't use RUSTC_BOOTSTRAP, you are not bootstrapping the compiler.

@sunshowers
Copy link

sunshowers commented Aug 26, 2019

With all due respect, @nox, as much as I agree with your object-level position — your comment (and the general style of your comments across these discussions) can be phrased in a more helpful manner. I've worked on developer tooling for many years, and one thing I've learned is that tools are for people to solve problems. If people are using a tool in unintended ways, the problem lies with the tool, not with the people. This is indeed one of the reasons Rust is so successful — it tries to be as accessible as possible instead of blaming its users for making mistakes.

At the very least it's worth understanding your users and trying to help them achieve their goals through other means. For example, in this case a test library available on crates.io wasn't used because it seemed to be too old. Maybe it's worth publishing a newer version of that library?

Thank you!

@nox
Copy link

nox commented Aug 26, 2019

This is indeed one of the reasons Rust is so successful — it tries to be as accessible as possible instead of blaming its users for making mistakes.

One of the biggest reasons Rust succeeded is that people can rely heavily on its stability promise that crates that work on stable will do so indefinitely (barring critical bugs). Using RUSTC_BOOTSTRAP through the build script to silently enable unstable features breaks this promise.

@sunshowers
Copy link

sunshowers commented Aug 26, 2019

There are also plenty of other ecosystem promises crates regularly break, such as unintended use of features (e.g. mutually exclusive features). Rust has succeeded despite those promises being broken left and right, so maybe it isn't that big a deal.

As I said, I agree with your position, but not with how you've expressed it in these discussions.

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 a pull request may close this issue.

3 participants