Visibility Trilemma. Solvable by new visibility keyword?

language design
1

I wrote an application (as opposed to a library) and I'm trying to integration test it via /test folder next to the src folder.

I tried the following options:

  • lib.rs with pub(crate) on all the API:s: compiler says that many of the API:s are never used even if they are used
  • lib.rs with pub on all the API:s: compiler: compiler fails to warn about unused API:s (and some other lints)
  • no lib.rs: the API:s are inaccessible to the /test folder (sibiling of /src), and also inaccessible to main.rs

To solve this issue, I think we should introduce a new visibility keyword (for example we may call it repo) that corresponds to the parent of crate (i.e. all siblings of src folder, including main.rs. Or if you have any other suggestions for how to fix the above issue, feel free to present your ideas. But in any case I think having to choose between different problems is an undesirable situation and preferably I should be able to have none of the above problems when creating integration tests.

This situation can be reproduced as follows:

  1. cargo new nice
  2. insert the following code:
`src/main.rs` 
use nice::functions::stringer;

fn main() {
    let stringer = stringer();
    dbg!(&stringer);
}
`src/functions.rs` 
pub fn stringer() -> String{
    format!("stringer")
}
`src/lib.rs` 
pub mod functions;
`test/tester.rs` 
#[test]
fn tester() {
    let stringer = stringer();
    dbg!(&stringer);
}
2

Integration tests are for the public API so your second approach is the correct one for them (test exported pub items). Use unit tests if you want to keep pub(crate). Test Organization

I agree however that it that current lints about unused APIs aren't very helpful in larger projects. It's a more general issue that also appears when splitting a project across multiple crates. I feel that having something to warn on unused items across crates based on some build target (or group of build targets) may be more useful than adding a new visibility qualifier. In particular, it would avoid a language change.

6 Likes
3

I've been in this situation several times; I always end up at "workspace with a library crate and a separate binary crate", and integration tests for the library crate.

As you say, this doesn't give any warning for either:

  • public items that are never tested by an integration test
  • public items that are never used by the binary crate, even though basically the library is only intended for use by that one binary crate

It would be nice if there was straightforward tooling to give warnings for either or both of these cases. My personal first thought would be a separate cargo command or cargo.toml option, rather than going in the Rust code itself.

(The former is a little like "code coverage", which I see there is tooling to check, but slightly different and "simpler/cleaner" because it only looks at the public API rather than lines of code inside the library crate)

1 Like
4

AFAIK this can only happen if they are used only when some cfg flags are enabled, and the solution is to also put those APIs behind the same cfg flags.

2 Likes
5

I get this issue even without using any cfg flags

6

I was under the impression that tests/ did not have visibility to pub(crate) items in lib.rs, as the intent is to test as-if you are a downstream crate. Am I misremembering?

7

if you put this in src/lib.rs:

pub(crate) mod functions;

then both /tests and /src/main.rs will have no visibility of src/functions.rs. But change it to just:

pub mod functions

then src/functions.rs becomes accessible to both /tests and /src/main.rs.

I added code in the OP if u wanna copy it over to your IDE and play around with it

8

If pub(crate) is not visible outside the crate, and you are claiming they are used...

... and not using cfg, then are they actually used? (by a function that is not also unused per the compiler).

I am confused by the statement of the problem in the OP, if the only way to test the code is pub then that seems like the best (only) option?

1 Like
9

This doesn't happen when modules are used correctly. I suspect you're using mod in your main.rs, so the same files are used twice, once included from main.rs, and once included from lib.rs. Because every mod instance is treated as completely separate new code, if you include the same file in both compilation units, you will get duplicated unused code and the compiler will be correct that there is an unused copy.

The solution is to put 99.9% of your application inside the library (lib.rs and its modules), don't use any mod in main.rs, and then just use main.rs to call the app from lib.rs.

4 Likes
10

yeah pub is the only option to test the code via /test but then the lints get weakened so that things I should change with the code won't be warned about (for example unused API:s)

11

I wonder why you want to test using integration tests. They are supposed to test only the public interface of your crate, if your tests need access to implementation details then you should use unit tests.

4 Likes
12

Sometimes integration tests are more suitable, for when you are testing the whole binary except argument parsing (or even including that sometimes).

A trick I have used is to put the top level function after argument parsing into lib.rs, and have main be a thin wrapper that does argument parsing and then calls the real main. As a bonus use doc(hidden), to make it clear it isn't intended for public use.

You could also execute the binary from the integration test, but unfortunately cargo doesn't make it easy to find the path to the built binary. Also executing the binary hundreds to thousands of times (lots of tests, short running command line program) has non-trivial overhead. Especially on Windows. Just calling a function is much faster, and since there is no global state, not an issue.

3 Likes
13

I'd like to add that some bugs occur only in some rare/unusual corner cases that won't be detected by just running the program. Integration-testing the binary would let me construct the specific combination of circumstances that previously have caused bugs, and I can put that in the CI to ensure the bug doesn't accidentally get reintroduced by another contributor.

1 Like
14

Independent of other factors in this discussion, one idea we've toyed with for open namespaces is a pub(namespace) scope. This may also apply to bin/lib/tests splits as well.

4 Likes