Pub(test) visibility modifier

I often find myself wanting a visibility modifier which makes the function pub for when compiled under test but not otherwise.

Motivation

I realize this wades into the white box vs black box testing debate and tests that test internal components should be unit tests and not integration tests. But sometimes it still feels like it would help. The situation where I most often encounter this is when I am testing a component which is a "middle" layer. (i.e.: It receives a public api call, does some computation and then calls some other component.) In such cases it makes sense to have tests that supply the implementation of the component that gets called into. This is easy enough to do, but it means that there is an additional API to supply an implementation that may or may not make sense to be public. (Depending on the crate)

What this might look like

If there were a visibility modifier pub(test) on a method:

pub(test) fn foo() { ... }

This would be roughly the same as:

#[cfg(test)]
pub fn foo() { ... }

#[cfg(not(test))]
fn foo() { ... }

Where the method foo is treated as pub when compiled under test and not pub when compiled normally.

Does this make sense? Do other people have this same problem/desire?

A somewhat similar pattern I am aware of in another language is that in Java it is common to make something public or package visible and annotate the method with @VisableForTesting to indicate to readers that the only reason for the increased visibility is for tests.

9 Likes

I think this doesn't fly from a grammar perspective for the same reason we have pub(in path) rather than pub(path). E.g. for

struct Foo(pub(in path) u32);

we want to be able to tell with bounded lookahead whether it's pub (parenthesized type) or pub(restricted) type. This only holds if the first item in the parenthesis is a keyword for pub(restricted) syntax.

Note ofc that if you have an in-module unit test, that has full visibility of things in scope; it's just if you need access in another module's unit tests (pub(crate) mostly serves this) or in the integration tests that you need to increase visibility in tests.

If there isn't already, someone should write a simple proc-macro that turns

#[test_pub]
$vis $item;

into

#[cfg(test)]
pub $item;

#[cfg(not(test))]
$vis $item;
5 Likes

Wow. I had no idea this was legal:

pub struct MyStruct(pub(u32,u32));

I assumed the space after pub was mandatory. (It obviously is if there are no parentheses).

The only place that whitespace is significant in the Rust grammar is in separating tokens. For all other intents and purposes, the parser completely ignores whitespace.

2 Likes

While I agree with @CAD97 that this could be done with a procedural macro, I think it would be better if this was possible without dependencies.

It would be nice if you could just write

pub(cfg!(test)) fn foo() {}

This would require the following:

  • Since cfg! expands to true or false, the grammar would need to be extended to allow pub(true) and pub(false)
  • macro invocations would have to be allowed in this place
1 Like

The ability to apply an arbitrary cfg to just the visibility modifier seems entirely reasonable.

In the interim, you might try pub(crate).

5 Likes

Oh, that is nice because it lets you do feature flag based visibility too.

Do you have some syntax in mind?

pub(test) is a new syntax, and it'll raise questions about when to use it vs #[cfg(test)].

How about making #[cfg(test)] "just work" instead?

I presume the library will have to compiled twice anyway to support pub(test), so it may be simpler to change Rust's policy of compiling non-test library for tests, and just compile everything with cfg=test when testing.

2 Likes

I didn't when I wrote that comment, but I'd imagine something like pub(#[cfg(...)] visibility).

1 Like

That idea crossed my mind too, but to make a function public only in tests, you'd have to write

pub(#[cfg(not(test))] self) fn ...

Which involves negative reasoning and is more complicated than my idea.

With the syntax I had in mind, the visibility was optional and defaulted to pub, so you could just write pub(#[cfg(test)]).

That also a nice solution. The syntax is a bit weird though, since attributes usually stand before something that is turned on/off.

As a slight variation on @josh's suggestion we could allow an otherwise redundant pub(pub) (alternatively you could think of naked pub be a shorthand for pub(pub)):

pub(#[cfg(test)] pub) fn foo { ... }
pub(#[cfg(test)] crate) fn bar { ... }
pub(#[cfg(test)] in some::path) fn baz { ... }

However, as a consequence this would mean pub() (with empty parentheses) is the equivalent of "not public" - but perhaps you could read it as "public nowhere".

3 Likes

So since (on 2018) pub(in must be followed by crate, self or super, can't pub(in test) or pub(in cfg(test)) (more general) be supported?

Edit: using another keyword to disambiguate would make this possible: pub(in super if cfg(test))

1 Like

The easiest solution would be to have an attribute which is similar to #[cfg_attr], except that it adds a visibility modifier instead of an attribute:

#[cfg_visib(test, pub)]
fn foo() {}

The advantage is that it doesn't need language changes, and anyone who knows #[cfg_attr] will understand how it works.

5 Likes

@Aloso Yeah, that does seem like the easiest solution. (Also, if it's going to be abbreviated, the common abbreviation for visibility is vis, so cfg_vis.)

1 Like

And already in use by the language via the $:vis macro matcher.

1 Like

Also it should be noted that within cfg(test) there is no difference between pub and pub(crate) since you never link against a crate compiled for test. So this is entirely about internal privacy to ensure you don't accidentally use something from a different module other than in unit tests if you were to just make it always pub(crate).

If something like this would be added I would like it to be more general than just for tests, though. E.g., for experiments, or temporary refactoring. E.g., some code that really should be private, but to run experiment A, or to facilitate temporary sister team project B, or ad-hoc customer request C, it needs to be accessible from external location Z.