Pants: Proposal: Move our unit tests from `tests/` to `src/`.

[benjyw]

As best I can tell, having a separate tests tree is a legacy of simplistic build tools such as Maven, that expect to bundle entire source trees and don't want to bundle tests along with the code.

Pants, however, does not suffer from this limitation, so we seem to be adhering to that style unnecessarily.

There are strong advantages to having unit tests (and maybe even all tests) live alongside the code they test:

• It's easier to find the corresponding tests when reading the code.
• It subtly encourages people to write more tests, as it's easy to see at a glance which source files are missing them.
• It showcases the fact that Pants supports this.

We've been doing this at Toolchain and it works really well.

I propose that we:

1. Require new unit tests to be sibling files of the code they test, with the suffix _test, so the tests for foo.py go in foo_test.py. Ordered directory listings then make it very easy to see which files have tests.

2. After a few weeks of seeing how we like it, consider requiring all new tests (including integration tests) to be in the src/ tree (with the suffix _integration_test.py).

3. Consider moving all existing unit tests (mechanically, of course) to src/.

Note that the default sources for python_library are *.py - (test_*.py, *_test.py), and the default sources for python_tests are (test_*.py, *_test.py), so this pattern is supported by these defaults.

Please comment with your opinions!

[jsirois]

Sounds good to me with the addition of Test.java, Test.scala, Spec.scala suffixes for the java (80 existing files) and scala (4 existing files) cases maybe after we complete phase 3 above for python.

[stuhood]

I'm willing to give this a try.

I love default sources, and strongly encourage their use. But I don't think they'll be much help here, because we have so many huge directories/modules. Having said that, that portion just seems like a continuation of the status quo (currently two directories containing lots of targets each, soon 1 directory containing lots of targets).

[benjyw]

Yeah, although I would like us to move towards one target per directory, and split directories where it makes sense to do so. But that's almost orthogonal to this.

[benjyw]

@jsirois The java/scala cases are already taken care of in default sources.

[jsirois]

Ack - I was just referring to an edit of your proposal. It talks about src/ but assumes src/python/

[cosmicexplorer]

I strongly appreciate the idea of colocating all tests with their sources, including integration tests. Rust happens to go one further and keep unit tests in the same file -- I don't think we would go that far, but having examples of using our python source code in the same directory as the source seems like a very good thing for potential and current contributors. This also avoids having directory trees in tests/ bitrot and start to diverge from src/ over time, which makes it difficult to know where to put new tests or find existing ones.

[benjyw]

Gotcha, yes.

[illicitonion]

My only hesitation here is that right now I have IntelliJ set up to treat source directories and test directories differently (e.g. it's easy to filter out test-only search results), and I would be sad to lose that, but as this layout is pretty common in the real world I assume IntelliJ has some fix here (though I couldn't find it at a quick glance).

[stuhood]

My only hesitation here is that right now I have IntelliJ set up to treat source directories and test directories differently (e.g. it's easy to filter out test-only search results), and I would be sad to lose that, but as this layout is pretty common in the real world I assume IntelliJ has some fix here (though I couldn't find it at a quick glance).

Mmm, yea, that is annoying. cc @wisechengyi

My understanding is that in IDEA, modules always own an entire directory. So in cases where 1:1:1 is not followed (...and this suggestion would decrease those), it gloms multiple targets together, which can create IDEA module cycles.

[jsirois]

Well, this is a very specific style of 1:1:1 breaking. If test targets never depend on each other and only on prod and prod never depends on test, I think you never get a cycle from this.

[benjyw]

@illicitonion Are you referring to JVM code? I use IntelliJ with the Python plugin (which is basically PyCharm under the IntelliJ skin) and AFAICT it doesn't have a "Tests" designation for Python source code directories (it does for JVM code though). I mostly care about our Python code right now, I don't mind punting on our JVM code.

[illicitonion]

@benjyw You can mark Python directories as a test sources directory. e.g. in this screenshot of the find dialog, the green backgrounded things are test things (or test infra), and the white backgrounded things are production things.

Find usages also groups by Production and Test, as per the other screenshot.

[benjyw]

Oh yes, I see now, you can designate test roots, but not individual test dirs.

[jsirois]

@illicitonion perhaps scopes are the right tool here?:

I never used them before, but just defined 'Python Tests' and the 'Python Code' (as the negation of 'Python Tests'). These scopes can be used in the find dialogs to, for example, filter out tests using the 'Python Code' scope.

[wisechengyi]

Sorry late to the party. One needs to mark a directory as test sources. If
the structure looks like src/test/Lang, then this needs to happen # of Lang
times.

On Sat, Apr 20, 2019, 7:50 AM John Sirois [e-mail] wrote:

@illicitonion https://github.com/illicitonion perhaps scopes are the
right tool here?:
[image: image]
https://user-images.githubusercontent.com/157586/56458865-050cd400-6349-11e9-8986-5c451ec6ed05.png

I never used them before, but just defined 'Python Tests' and the 'Python
Code' (as the negation of 'Python Tests'). These scopes can be used in the
find dialogs to, for example, filter out tests using the 'Python Code'
scope.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/pantsbuild/pants/issues/7489#issuecomment-485133303,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAERTBXBOINVSI3XG6KITSTPRMUS7ANCNFSM4HDLU5OQ
.

[benjyw]

Sorry late to the party. One needs to mark a directory as test sources. If the structure looks like src/test/Lang, then this needs to happen # of Lang times.

The idea is that there would be no test directories. Tests would be siblings of the code they test.

[illicitonion]

@illicitonion perhaps scopes are the right tool here?:

I never used them before, but just defined 'Python Tests' and the 'Python Code' (as the negation of 'Python Tests'). These scopes can be used in the find dialogs to, for example, filter out tests using the 'Python Code' scope.

Scopes look indeed like they can do what I want - I had no idea they existed. Thanks!

[benjyw]

Oooh, that is a nifty feature!

[Eric-Arellano]

This has mostly been done for V2 code. V1 code is labeled as wont-fix (unless anyone wants to spend the time on it).