Expand ArchUnit Testing

[nrmancuso]

While ArchUnit was introduced in #7469, we only did very basic initial integration (see #8112). We would like to expand our usage of ArchUnit.

It would be a good first step to see if we can complete the work started at #8223, without having ArchUnit add generated files to the project.

References:
https://blog.jdriven.com/2018/10/testing-the-architecture-archunit-in-practice/
https://www.mobileit.cz/Blog/Pages/arch-unit-1.aspx
https://www.mobileit.cz/Blog/Pages/arch-unit-2.aspx
https://www.mscharhag.com/software-development/architecture-validation-with-archunit

Good "alternative" uses:
https://nexocode.com/blog/posts/archunit-forget-architecture-it-s-a-flexible-and-intelligent-linter/

Let's use this issue to discuss usage ideas.

[nrmancuso]

Here might be a good candidate for ArchUnit, if we decide to take action on this comment:
#12100 (comment)

Forbidding mutable test fields.

[rnveach]

I would consider a test field immutable only if it's value never changes throughout the life of the test run, including the place where it got its value from. If I can get 2 different values from the same method call when executing it at the start of the test run and any time between then and the end of the test run, then the field isn't immutable.

[Vyom-Yadav]

Forbidding mutable test fields.

Using ArchUnit, we can only make sure that fields are final, i.e. something like-

@ArchTest                                                                
public static final ArchRule NEW_RULE = fields().that()                   
    .areDeclaredInClassesThat()                                           
    .haveSimpleNameEndingWith("Test")                                     
    .should()                                                             
    .beFinal();                                                          

[Vyom-Yadav]

It would be a good first step to see if we can complete the work started at #8223, without having ArchUnit add generated files to the project.

The added files by ArchUnit are due to https://www.archunit.org/userguide/html/000_Index.html#_freezing_arch_rules, if we are looking to integrate it, this can act as a suppression file as it will contain all the violations, another method can be to create custom slices without some classes, ignoring those classes, so no violations will be received from them.

[Vyom-Yadav]

@nick-mancuso @rnveach Let's approve this.

[Vyom-Yadav]

ArchRules to be implemented:

• 1. Partial Immutability check: This ArchRule is divided into 2 parts.
  1. Util classes immutability check: This check makes sure that all fields defined in Util classes are final and static and immutable. If the field contains a generic type, it is checked that the generic type is immutable.
  2. Stateless Module check: This check makes sure that all fields defined in stateless modules that are not properties are final and are immutable. Also every field other than message keys should be private. If the field contains a generic type, it is checked that the generic type is immutable. This also makes sure that modules with immutable fields are annotated with @StatelessCheck and modules with mutable fields are annotated with @FileStatefulCheck or @GlobalStatefulCheck.

These ArchRules cannot check reference immutability.

• 2. Disallow classes in api package to depend on classes in util package

• 3. Validation of test/input package/file structure for AST regression
  Edit from @nrmancuso : let's skip (3), as we have non-compilable files too, so we wouldn't be able to check them using this method.

• 4. Checks are extending only AbstractCheck or AbstractFileSetCheck or AbstractJavadocCheck

• 5. All checkstyle packages are free of cycles

• 6. Checks have all private fields.

• 7. No check subpackage should depend on each other.

[Vyom-Yadav]

PR for Partial Immutability check: #12135

[romani]

all changes are merged, if something else required, open new issue,

[rnveach]

2 items were not done at #12099 (comment)

[romani]

Checks have all private fields.

I pretty sure it is covered by our modifier Check.

. No check subpackage should depend on each other.

It is covered by jarch cycle validation

[rnveach]

Checks have all private fields.
I pretty sure it is covered by our modifier Check.

Which one https://checkstyle.org/config_modifier.html ?

No check subpackage should depend on each other.
It is covered by jarch cycle validation

Cycle is 2 way. Package 1 > Package 2 > Package 1. This was 1 way. Package 1 > Package 2. It was to represent checks don't rely on other checks.

[romani]

https://checkstyle.org/config_design.html#VisibilityModifier

was to represent checks don't rely on other checks.

I think we need to practice existing cycles tests to find how to make more advanced validation.