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

make code compiled with cfg(test) visible across crates #8379

Open
toxeus opened this issue Jun 18, 2020 · 9 comments
Open

make code compiled with cfg(test) visible across crates #8379

toxeus opened this issue Jun 18, 2020 · 9 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-test S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@toxeus
Copy link

toxeus commented Jun 18, 2020

I find myself creating test helpers/mocks in my test modules that are marked with #[cfg(test)]. They are exported as pub and can conveniently be used throughout the crate.
These helpers are not visible from other crates nor from integration tests in particular.

Making code compiled with cfg(test) be visible outside of the scope of a crate would allow for better code reuse. Please consider adapting that behavior.

Related: rust-lang/rust#45599

@toxeus toxeus added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jun 18, 2020
@ehuss ehuss added Command-test E-hard Experience: Hard labels Jun 25, 2020
@TomzBench
Copy link

I have a util crate that exports some mocks. Real code imports a real routine from the crate and test code imports a mock routine from the crate. However this does not work!

Is there a way to implement this or do I have to stuff everything with a mock in the same crate?

As work around I declare the mock in the crate that needs the mock, but then I have to duplicate this mock in every crate :(

@oldgalileo
Copy link

As work around I declare the mock in the crate that needs the mock, but then I have to duplicate this mock in every crate :(

Did you ever come up with or find a more ergonomic solution to this? After running into this, I've been very surprised at this being a largely unresolved issue. It seems like it would be more of a common practice to, for example, provide a once_cell::sync::Lazy initialization of cool_library::InterestingStruct which has private fields so that you don't need to compromise good visibility hygiene for testability.

So far, I've seen suggestions for mock crates, or a "testing" feature-flag with #[cfg(not(testing))] -> panicking. These seem like hacks.

@oeed
Copy link

oeed commented Sep 29, 2022

While this would be nice to have, you can replicate this in a less nice way using crate features. On your utils crate, add a feature, e.g. test-utils, tag modules with #[cfg(feature = "test-utils")] and then on your consumer crate add:

[dev-dependencies]
my-crate = {path = "../my-crate", features = ["test-utils"]}

@simsekgokhan
Copy link

simsekgokhan commented Oct 16, 2022

Also facing this problem, thx for the "feature" workaround. This is how I used the workaround if anybody needs: 0LNetworkCommunity/libra-legacy-v6@6106c07, look for "ol-smoke-test".

imotov added a commit to imotov/quickwit that referenced this issue Feb 10, 2023
fulmicoton added a commit to quickwit-oss/quickwit that referenced this issue Feb 15, 2023
* Catch violations in times(...) expectations

Adds a new quit() method to universe that causes all spawned actors
to quit. It also collects actors exit codes that can be used to verify
that all actors quit gracefully, which in turn allows to fail the test
if mock objects expectations are not met.

Relates to #2754

* Make clippy and rustfmt happy

* Fix test_indexer_trigger_on_memory_limit

* Update quickwit/quickwit-actors/src/actor_handle.rs

Improve panic handling

Co-authored-by: Paul Masurel <[email protected]>

* Add universe.assert_quit() method

Also adds a check to the universe.drop() to make sure that
universe.assert_quit() was indeed called at the end of the test.

* Stop test_ingest_api_source tests from hanging.

* Fix compilation errors when testing a single crate

Seems like rust-lang/cargo#8379 is the issue
here.

* Update assert_quits after merge

* Avoid using map with side-effects.

Co-authored-by: Paul Masurel <[email protected]>

* Switch ActorJoinHandle to Shared

* Explain why we join ingest source first

* There should be only one universe.

* Fix format

---------

Co-authored-by: Paul Masurel <[email protected]>
@weihanglo weihanglo added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label May 15, 2023
@ryoqun
Copy link

ryoqun commented Jun 9, 2023

While this would be nice to have, you can replicate this in a less nice way using crate features. On your utils crate, add a feature, e.g. test-utils, tag modules with #[cfg(feature = "test-utils")] and then on your consumer crate add:

[dev-dependencies]
my-crate = {path = "../my-crate", features = ["test-utils"]}

I'm seriously thinking about coping with the limitation described in the issue... @oeed this work around looks nice but i think it's affected by cargo's feature unification, right?
in my testing, it seems that other member crates of my workspace are seeing #[cfg(feature = "test-utils")] -ed code even without proper dev-dependencies and narrowing down cargo check with -p foo actually causes expected compile errors...

@jaques-sam
Copy link

While this would be nice to have, you can replicate this in a less nice way using crate features. On your utils crate, add a feature, e.g. test-utils, tag modules with #[cfg(feature = "test-utils")] and then on your consumer crate add:

[dev-dependencies]
my-crate = {path = "../my-crate", features = ["test-utils"]}

How to enable the feature for a integration test?

robinhundt added a commit to encryptogroup/SEEC that referenced this issue Jan 23, 2024
I found a way to work around
rust-lang/cargo#8379
zanieb added a commit to astral-sh/ruff that referenced this issue Feb 1, 2024
Updated implementation of #7369
which was left out in the cold.

This was motivated again following changes in #9691 and #9689 where we
could not test the changes without actually deprecating or removing
rules.

---

Follow-up to discussion in #7210

Moves integration tests from using rules that are transitively in
nursery / preview groups to dedicated test rules that only exist during
development. These rules always raise violations (they do not require
specific file behavior). The rules are not available in production or in
the documentation.

Uses features instead of `cfg(test)` for cross-crate support per
rust-lang/cargo#8379
@ryoqun
Copy link

ryoqun commented Jul 2, 2024

While this would be nice to have, you can replicate this in a less nice way using crate features. On your utils crate, add a feature, e.g. test-utils, tag modules with #[cfg(feature = "test-utils")] and then on your consumer crate add:

[dev-dependencies]
my-crate = {path = "../my-crate", features = ["test-utils"]}

I'm seriously thinking about coping with the limitation described in the issue... @oeed this work around looks nice but i think it's affected by cargo's feature unification, right? in my testing, it seems that other member crates of my workspace are seeing #[cfg(feature = "test-utils")] -ed code even without proper dev-dependencies and narrowing down cargo check with -p foo actually causes expected compile errors...

FYI, I implemented this with the quoted concern being addressed. If interested, please look for dev-context-only-utils at https://github.com/anza-xyz/agave

It seems dev-context-only-utils is working as expected over 1 year at the moderate cost of increased CI to detect any misuse of the special feature.

Note that I recently encountered a minor problem: #10958, see extended explanation as well: #10958 (comment)

In near (?) future, I'm going to publish cargo-dcou or something for ease of use...

@HatemMn
Copy link

HatemMn commented Nov 21, 2024

Hello, any news on this ? I also use the features = ["test-utils"] method for now but It would be cool that cgf=test could be exported

@epage epage removed the E-hard Experience: Hard label Nov 21, 2024
@epage
Copy link
Contributor

epage commented Nov 21, 2024

As tagged, this needs design work to move forward. The problem here is that cfg(test) is only applied to test binaries. For unit tests, we build a separate copy of the library as a test binary.

If we always built the library with --cfg test then we'd be building a second copy of it, unable to reuse the original. This would increase build times for people who don't need this.

This also has people muddy the water between white box tests (in the lib) and black box tests (in tests/) and having special access to the lib shouldn't be a default path.

imo the best route forward is the self-enabled feature while making sure to do test-compiles without dev build targets enabled to catch accidentally relying on this behavior. In the short term, I'd prefix the feature with unstable- and in the long term, I hope we get private features (there is an issue for this). We might want to find a way to document this pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-test S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
Status: No status
Development

No branches or pull requests