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

How to relax the rules for tests #1

Closed
JensPiegsa opened this issue Apr 26, 2017 · 6 comments
Closed

How to relax the rules for tests #1

JensPiegsa opened this issue Apr 26, 2017 · 6 comments
Assignees
Milestone

Comments

@JensPiegsa
Copy link

ArchUnit is a great testing framework! I wonder whether there is a good way to relax the rules for classes under /src/test/java in a portable way.

@leimer
Copy link

leimer commented Apr 26, 2017

Hi Jens, do you want to just disable all test rules for classes in src/test/java or disable some of your rules?

@JensPiegsa
Copy link
Author

Hi Johannes,

I think both make sense.

  1. To keep it simple it should be possible to completely take test code out of the equation.
  2. Advancing further one especially might want to introduce assertions about how unit and integration tests are written.

@codecholeric
Copy link
Collaborator

Hi Jens,

first of all, thanks for the feedback and asking the question :-)

I've thought about this in the past, as well. Unfortunately, I haven't come up with the silver bullet for this. In the end, all the class file import knows, is the file URL of a class, which is in general not enough, to know if a class is test code.

However, I did include a best guess for Maven and Gradle (as well as IntelliJ Idea). It uses the fact, that Maven compiles test classes to target/test-classes and Gradle compiles classes to build/classes/test (at least the versions I've checked).

The ClassFileImporter allows to specify ImportOptions, which are pretty much just filters on URLs. If you use the JUnit style to write tests, you can thus write

@AnalyzeClasses(..., importOption = ImportOption.DontIncludeTests.class)
public class MyTest{
    // ...
}

This will simply exclude all URLs that contain '/test-classes/' or '/test/', which should leave exactly production code (except if you put your whole project in a folder called 'test', as I said, it's just a best guess). If this is not good enough, you can just implement ImportOption, tailor it specifically to your setup, and refer to it from @AnalyzeClasses.

So you can put your tests for test code and your tests for production code in two different test classes and use different import options.

Is this good enough for your usecase?

NOTE: I've just noticed, that I was a little sloppy on the implementation of DontIncludeTests.class and hardcoded the *nix file separator. This is fixed on master, but if you're developing on Windows, the provided ImportOption of version 0.4.0 will probably not work correctly. If so, you could just copy DontIncludeTests from https://github.com/TNG/ArchUnit/blob/master/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java and use that for your tests.

@codecholeric codecholeric self-assigned this Apr 26, 2017
@JensPiegsa
Copy link
Author

Hi Peter,

thanks for your thoughts on this subject. I looked here: ArchRule rule = noClasses().that().|, but I understand, that there is no information about the origin of the class file at this point.

The ImportOption filters look promising. With @AnalyzeClasses you can only specify one importOption, but I'll try to call ClassFileImporter with DontIncludeJars and DontIncludeTests . Thanks for mentioning the file separator issue.

@JensPiegsa
Copy link
Author

JensPiegsa commented Apr 27, 2017

Ok, I got it working. :-)

private final JavaClasses classes = new ClassFileImporter()
		.importClasspath(new ImportOptions()
				.with(new ImportOption.DontIncludeJars())
				.with(new ImportOption.DontIncludeTests()));

Two small suggestions:

  • the new check with File.separator is not supposed to work, as Location.contains() checks the URI, and this should always contain forward-slashes /
  • you might want to be more strict about the checked URI path segments. How about /build/classes/test/ and /target/test-classes/

@codecholeric
Copy link
Collaborator

Aaaaaaah, you're absolutely right, now I remember, I wasn't sloppy about the file separator, I though "thank God we're dealing with URIs here and don't have to worry about the file separator" *facepalm*.
Thanks for paying attention, shouldn't do those things at midnight... :-( Especially with platform dependent stuff, where I don't have any test environment.

Anyway, glad to hear it works for you now :-) If you want to use @AnalyzeClasses for the caching, you can still just make your own ImportOption, that just returns dontImportTests.includes(location) && dontImportJars.includes(location).

Nevertheless, this got me thinking that the behavior could be more consistent. Thus from 0.5.0 on, importOption in @AnalyzeClasses will be an array importOptions (even though this will be a breaking change, hope the easy migration will make it acceptable).

I've also applied your suggestion, DontIncludeTests is now looking specifically for /build/classes/test/ and /target/test-classes/, which should make it less confusing, if some folder test exists somewhere within the project location.

Since you say you got it working, I'll pull this change in and close this issue :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants