futures: rework tests/ to work under all --feature combinations #2101

kentfredric · 2020-03-19T06:25:39Z

This patch set extensively restructures the tests to:

Make import ownership of individual tests more apparent
Simplify feature gating individual tests
Simplify potential future test set fragmentation
Execute all tests that can be expected to compile under a given configuration.

With this patch set, one can now do:

 cargo -q hack \
      --each-feature \
      --skip bilock \
      --skip cfg-target-has-atomic \
      --skip read-initializer \
      test -q --tests

 cargo -q hack \
      --features unstable \
      --each-feature \
      --skip unstable \
      test -q --tests

 cargo -q hack \
      --feature-powerset \
      --skip bilock \
      --skip cfg-target-has-atomic \
      --skip read-initializer \
      test -q --tests

 cargo -q hack \
      --features unstable \
      --feature-powerset \
      --skip unstable \
      test -q --tests

And experience no compilation failures, or compilation warnings.

( There's a bit of redundancy in my examples as --feature-powerset is a superset of --each-feature, but --each-feature is better for doing initial discovery of missing features, and a separation must be made for the 3 features that require "unstable" turned on )

Nothing in this PR addresses either the internal tests ( test --lib ) or the doc tests ( test --doc ) as they're both more contentious and difficult to do, and I'd rather see this PR merged first before embarking on shoring up those problems.

And it also may be worth adding these cargo hack invocations into the standard CI tests to ensure these features continue to be adequately tested.

Tips: Due to the approaches used for testing, this can cause target/debug/ to become incredibly large, and prone to running you out of disk space if you're resource constrained.

I've personally taken to throwing

 CARGO_INCREMENTAL=0 RUSTFLAGS="-C debuginfo=0"

Into things, which cuts my target/debug dir down from 4.5GB for the first cargo hack command, down to 2.6GB.

Obviously this is a provisional PR and I can rebase like a champion, so any changes that would be required to make this merge welcome.

All tests in this file use both "alloc" and "executor" features. Subsequently, all the tests are inherently disabled without both these features turned on. "use" declaration interned into their various tests to make it cleaner to identify what tests are consuming which functionality, and keep the number of `#[cfg()]` blocks to a minimum. Tests now pass with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test abortable cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test abortable cargo hack --features "unstable" --each-feature \ test --test abortable cargo hack --features "unstable" --feature-powerset \ test --test abortable

These tests all use 'alloc'. I could have just blocked this whole file out with a file global #![cfg(feature=alloc)] However, in consideration of future proofing, I instead opted to reduce sharing of common logic so this test can be more clearly updated in future, while also minimizing the risk of warnings about unused things. Therin, the individual test cases control the required features instead of the required features being controlled at the file level. Tests now pass with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test arc_wake cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test arc_wake cargo hack --features "unstable" --each-feature \ --skip unstable \ test --test arc_wake cargo hack --features "unstable" --feature-powerset \ --skip unstable \ test --test arc_wake

…ures Tests in this file rely on a complex interaction of features. Subsequently, all tests are restructured to import their own dependencies, and then each test is cfg gated based on the features they inherently need. In 2 cases, common code definitions were moved into the test itself to avoid potentially complex code simply to avoid 'unused' warnings. These tests now pass with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test async_await_macros cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test async_await_macros cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test async_await_macros cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test async_await_macros

This test file has only one test, and it requires feature = executor Subsequently, all the imports have been moved inside the test to guard against accidental blind extension, and the test itself is gated with a '#[cfg()]` These tests now pass with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test atomic_waker cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test atomic_waker cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test atomic_waker cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test atomic_waker

…eatures The one test in this file requires all of: - alloc - std - executor To run, so this test is omitted without all these features present. Imports moved to inside the test case to discourage blind test extension, and to simplify avoidance of "unused" warnings. Tests now compile with the following: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test buffer_unordered cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test buffer_unordered cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test buffer_unordered cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test buffer_unordered Tests also *usually* pass with a variation of the above when passed cargo hack <...> test --test buffer_unordered \ -- -Z unstable-options --include-ignored However, I still personally see lockups occurring for no obvious reason as per rust-lang#1790

Some of these tests rely on the presence of the "alloc" feature. So I've reworked the imports to make ownership clearer, and reworked some tests to localise shared code, and then `#[cfg()]` gated out tests when their needed features aren't present. These tests now pass with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test eager_drop cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test eager_drop cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test eager_drop cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test eager_drop

This test requires both executor and thread-pool enabled to function. But as the code is all heavily shared and intertwined, it seems the most sensible thing to do is just file-level disabling. Spurious build failures no longer occur with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test eventual cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test eventual cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test eventual cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test eventual

Tests in this file use varying combinations of "alloc" and "executor" features. Subsequently, all the imports have been relocated to inside the test functions to make ownership clear, and easily isolate the needed features, and tests have been appropriately gated with `#[cfg()]` blocks. Tests now avoid spurious compile failures with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test futures_ordered cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test futures_ordered cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test futures_ordered cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test futures_ordered

…/o futures Various tests in this file require "alloc" functionality, "executor" functionality, or both. Imports relocated into relevant tests for clarity of ownership and ease of gating, and then tests are gated off with relevant `#[cfg()]` attributes. No spurious build failures (or test failures) are now seen with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test futures_unordered cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test futures_unordered cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test futures_unordered cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test futures_unordered

Two of the tests in this file are dependent on the "executor" feature. All imports reorganized into their nearest proximal necessary location to make import ownership clearer, and gated tests out by feature requirements with `#[cfg()]` No spurious build errors or test failures now with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test future_try_flatten_stream cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test future_try_flatten_stream cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test future_try_flatten_stream cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test future_try_flatten_stream

- imports relocated inside the test unit - gated test unit with appropriate `#[cfg()]` Tests compile and pass with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test inspect cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test inspect cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test inspect cargo hack --features "unstable" \ --feature-powerset \ --skip unstable \ test --test inspect

Various tests in this file need either or both "std" and "executor" Imports and symbols pushed down as low as possible to make symbol ownership more obvious, and also make gating tests against features easier. Tests appropriately gated with `#[cfg()]` Tests now compile and pass where possible with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_buf_reader cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_buf_reader cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test io_buf_reader cargo hack --features "unstable" \ --feature-powerset \ --skip "unstable" \ test --test io_buf_reader

Tests in this file use combinations of "std" and "executor" features. Imports and symbols moved to deeper lexical scopes to make owernship clearer, and to facilitate easy feature gating. Tests gated as appropriate with `#[cfg()]` Tests now compile and pass with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_buf_writer cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_buf_writer cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test "io_buf_writer" cargo hack --features "unstable" \ --feature-powerset \ --skip "unstable" \ test --test "io_buf_writer"

All the tests in this file consume "std" and "executor" features. As a future proofing step, and to clarify import ownership and simplify feature gating, imports have been pushed into the tests themselves. And all tests have been individually gated with `#[cfg()]`. Tests now compile and pass (where compiled) with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_cursor cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_cursor cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test io_cursor cargo hack --features "unstable" \ --feature-powerset \ --skip "unstable" \ test --test io_cursor

Tests in this file require either "executor" or "std" features. Imports and test related symbols pushed to deep isolation to simplify owership and test gating. Tests are gated as appropriate with `#[cfg()]` Tests now compile and pass (where relevant) with: cargo hack --each-feature \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_lines cargo hack --feature-powerset \ --skip bilock \ --skip cfg-target-has-atomic \ --skip read-initializer \ test --test io_lines cargo hack --features "unstable" \ --each-feature \ --skip "unstable" \ test --test io_lines cargo hack --features "unstable" \ --feature-powerset \ --skip "unstable" \ test --test io_lines

Reflow test to move imports into test function, and then gate entire test function on "executor" feature. cargo test --test io_read_exact Now compiles and passes in every valid combination of features ( see previous commits by me in tests/ for the command ) i

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

This whole test file is simple enough, and uses only the one import, so the whole file has been gated out when "std" is missing via a file-global `#![cfg()]` Tests now build and pass with all valid feature combinations See previous commits for details and rationale

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

@taiki-e

Fortunately, the author of this test used the same style I've been employing in my other patches to tests/ Therefor, fixing this to work properly without the "std" feature is a simple one line change. Tests now pass for this in all valid feature combinations. Hat-tip to @taiki-e :)

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

Only really appropriate action here is to gate the whole file with `#![cfg()]` Tests now build and pass with all valid feature combinations See previous commits for details and rationale

cramertj · 2020-04-03T20:09:00Z

This is awesome! Thank you so much for all this work!

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101.

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports.

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports in tests.

After #2216, these redundant imports are unneeded. This reverts almost all of #2101. This also includes some minor cleanup of imports in tests.

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports in tests.

After #2216, these redundant imports are unneeded. This reverts almost all of #2101. This also includes some minor cleanup of imports in tests.

kentfredric added 11 commits March 19, 2020 17:22

kentfredric mentioned this pull request Mar 19, 2020

futures: enable testing without --features thread-pool #2098

Closed

kentfredric added 18 commits March 19, 2020 21:00

futures: tests/io_read_line.rs: Don't break w/o features

89fcf19

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/io_read.rs: Don't break w/o "std"

c6807c1

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/io_read_to_string.rs: Don't break w/o features

3898293

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/io_read_until.rs: Don't break w/o features

7475a90

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/io_write.rs: Don't break w/o features

cb9e2ae

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/join_all.rs: Don't break w/o "executor" feature

f08ccb2

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/macro_comma_support.rs: Don't break w/o features

1e37927

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/mutex.rs: Don't break w/o features

cda90ce

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/oneshot.rs: Don't break w/o "alloc"

cf3929d

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/ready_queue.rs: Don't break w/o features

80011bb

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/ready_queue.rs: Don't break w/o "executor"

086b831

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

kentfredric added 14 commits March 19, 2020 21:00

futures: tests/select_all.rs: Don't break w/o "executor"

67cc501

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/select_ok.rs: Don't break w/o "executor"

320d123

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/shared.rs: Don't break w/o features

f39481c

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/sink_fanout.rs: Don't break w/o features

450221d

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/sink.rs: Don't break w/o features

8cc87d1

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/split.rs: Don't break w/o "executor"

7651ede

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/stream_catch_unwind.rs: Don't break w/o "executor"

5972712

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/stream_into_async_read.rs: Don't break w/o "std"

1f7eb52

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/stream_peekable.rs: Don't break w/o "executor"

02ad75a

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/stream.rs: Don't break w/o "executor"

0e54f94

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/stream_select_all.rs: Don't break w/o features

da0ff8b

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/stream_select_next_some.rs: Don't break w/o features

f7e2880

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/try_join_all.rs: Don't break w/o "executor"

59a2ac4

- Symbols/Imports pushed down / localised - Tests gated with cfg() - Tests now build and pass with all valid feature combinations See previous commits for details and rationale

futures: tests/try_join.rs: Don't break w/o features

e18ee1c

Only really appropriate action here is to gate the whole file with `#![cfg()]` Tests now build and pass with all valid feature combinations See previous commits for details and rationale

kentfredric force-pushed the pr-futures-tests-feature-rework branch from f13b412 to e18ee1c Compare March 19, 2020 08:01

cramertj merged commit c5852d3 into rust-lang:master Apr 3, 2020

taiki-e mentioned this pull request Jun 20, 2020

Option to clean between each run taiki-e/cargo-hack#47

Closed

taiki-e mentioned this pull request Sep 22, 2020

futures: Require all stable features in tests #2216

Merged

taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Feb 26, 2021

Revert import changes in tests

eb89d24

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101.

taiki-e mentioned this pull request Feb 26, 2021

Revert import changes in tests #2366

Merged

taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Feb 26, 2021

Revert import changes in tests

1771dd8

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101.

taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Feb 26, 2021

Revert import changes in tests

dd58252

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports.

taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Feb 26, 2021

Revert import changes in tests

4c121d2

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports in tests.

taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Feb 26, 2021

Revert import changes in tests

25cab70

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports in tests.

taiki-e added a commit that referenced this pull request Feb 26, 2021

Revert import changes in tests

cdb3d08

After #2216, these redundant imports are unneeded. This reverts almost all of #2101. This also includes some minor cleanup of imports in tests.

exrook pushed a commit to exrook/futures-rs that referenced this pull request Apr 5, 2021

Revert import changes in tests

a6ef72b

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports in tests.

taiki-e added a commit to taiki-e/futures-rs that referenced this pull request Apr 10, 2021

Revert import changes in tests

d723ae0

After rust-lang#2216, these redundant imports are unneeded. This reverts almost all of rust-lang#2101. This also includes some minor cleanup of imports in tests.

taiki-e added a commit that referenced this pull request Apr 10, 2021

Revert import changes in tests

9d69835

After #2216, these redundant imports are unneeded. This reverts almost all of #2101. This also includes some minor cleanup of imports in tests.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

futures: rework tests/ to work under all --feature combinations #2101

futures: rework tests/ to work under all --feature combinations #2101

Uh oh!

kentfredric commented Mar 19, 2020

Uh oh!

cramertj commented Apr 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

futures: rework tests/ to work under all --feature combinations #2101

futures: rework tests/ to work under all --feature combinations #2101

Uh oh!

Conversation

kentfredric commented Mar 19, 2020

Uh oh!

cramertj commented Apr 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants