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

Configuration get's mixed up in multi module build #37

Closed
1 of 3 tasks
nberger-qs opened this issue Aug 7, 2020 · 7 comments
Closed
1 of 3 tasks

Configuration get's mixed up in multi module build #37

nberger-qs opened this issue Aug 7, 2020 · 7 comments

Comments

@nberger-qs
Copy link

Summary

When using the arch-unit-maven-plugin in a multi module build together with archunit.properties file for each module, only the archunit.properties of the first module is loaded and used for all modules.

Type of Issue

It is a :

  • bug
  • request
  • question regarding the documentation

Motivation

I try to use ArchUnit in a multi module build where some tests are using FreezingArchRule. On the first run they all should "freeze" the current violations to a store. One for each module as only the classes for that module are tested.

Current Behavior

When running a multi module build with the plugin activated in multiple modules, the ArchUnit tests for the first module read the archunit.properties for that module and create a new store for the freezed violations. The ArchUnit tests of the second module do not create their own new store but the existing one of the first module. This store is then emptied as the violations stored in it do not exist in the second module and the violations of the second module are not stored but cause the build to fail.
Without setting the freeze.store.default.path the store is placed in the parent module of the two modules.
I explicitly set the freeze.store.default.path to ${project.basedir/archunit_store to force the usage of a separate store for each module. But then the store is just placed n the first module but still used for every module. Apparently only the ArchUnit tests of the first module run by the plugin do read the configuration and those of the next module don't read their own configuration but use the one of the first.

When running the same ArchUnit tests as JUnit5 tests using the maven-surefire-plugin, every module uses it's own archunit.properties. But that way causes other inconveniences as it is harder to reuse the same rules for multiple modules and mixes architecture tests with unit tests.

Expected Behavior

For each module in a multi module build, its own archunit.properties and a separate store for freezing should be used.

Steps to Reproduce (for bugs)

my archunit.properties in src/test/resources of each module (filterung in activated):

freeze.store.default.path=${project.basedir}/archunit_store
freeze.store.default.allowStoreCreation=true

My plugin configuration in the parent pom:

...
<modules>
  <module>mod1</module>
  <module>mod2</module>
</modules>
...
<build>
  <testResources>
    <testResource>
      <directory>src/test/resources</directory>
      <filtering>true</filtering>
    </testResource>
  </testResources>
  ...
  <plugins>
    ...
    <plugin>
      <groupId>com.societegenerale.commons</groupId>
      <artifactId>arch-unit-maven-plugin</artifactId>
      <version>2.4.0</version>
      <configuration>
        <!--force current "target" as project path for submodules -->
        <projectPath>${project.build.directory}</projectPath>
        <excludedPaths>playground</excludedPaths>
        <rules>
          <preConfiguredRules>
            ...
          </preConfiguredRules>
          <configurableRules>
            <configurableRule>
              <!-- ArchUnit test using freezing -->
              <rule>my.project.architecture.MainClassesArchTest</rule>
              <applyOn>
                <scope>main</scope>
              </applyOn>
            </configurableRule>
            <configurableRule>
              <rule>my.project.architecture.TestClassesArchTest</rule>
              <applyOn>
                <scope>test</scope>
              </applyOn>
            </configurableRule>
          </configurableRules>
        </rules>
      </configuration>
      <executions>
        <execution>
          <phase>test</phase>
          <goals>
            <goal>arch-test</goal>
          </goals>
        </execution>
      </executions>
    </plugin>
    ...
  <plugins>
  ...
</build>

Your Environment

  • Version used: 2.4.0
  • OS and version: Windows 10
  • Version of libs used: Java 8, Maven 3.6.3, archunit-junit5 0.14.1
@vincent-fuchs
Copy link
Contributor

I am not familiar with FreezingArchRule as it's part of the ArchUnit core. I've never used it myself.

@codecholeric do you have an idea about this ? is an archunit.properties file in each sub module supposed to work ?

@agtadadik
Copy link

Hi, did you find any solution for this? I am also facing the same issue and its important for me to use FreezingArchRule feature.

@vincent-fuchs
Copy link
Contributor

OK, so I tested this a bit, and I really think an issue should be open in https://github.com/TNG/ArchUnit because I don't think the problem is with the Maven plugin.

Here's what I did in my multi-module project :

  • I removed arch-unit-maven-plugin
  • in 2 modules (service and infra) , I have :
    • added an arch-unit test that catches a violation in the module
    • added a archunit.properties file to enable the recording of violations to freeze

and here's what happens :

  • 2 violation stores get created, one in each module
  • service that gets built first, contains one violation, the one from its module
  • infra that gets built second, contains 2 violations :
    • the one from its own module --> OK, it's expected
    • the one from service --> not exactly expected, right ?

is it also what you observe in your case ? can you try to replicate without using the plugin, like I did ?

@agtadadik
Copy link

No. I get only 1 violation in second module which belongs to that module only. So its as expected.

@agtadadik
Copy link

Also, rule stores are created correctly if I use maven-surefire-plugin.
I created a jar containing arch unit rule's library and then gave that as dependenciesToScan to maven-surefire-plugin. But for this to work I have to add arch unit test class in each module and then specify packages to analyze.

@vincent-fuchs
Copy link
Contributor

OK, I think I've been able to reproduce the issue you're experiencing.. right now, I have no clue "why" though, so not sure if/when I can fix it..

@vincent-fuchs
Copy link
Contributor

vincent-fuchs commented Mar 6, 2021

I investigated, and found out few interesting things. As anticipated, there's not much we can do in the plugin which is mostly a wrapper around ArchUnit.

By looking in ArchUnit code and in TNG/ArchUnit#389 (comment) , we find that :

  • the archunit.properties content is loaded once and cached : that's why we see the behavior here.
  • however, we can modify the properties values that are cached. So I think the trick is to set the path to the violation store in the cached properties before you execute the rule :
  1. Keep an archunit.properties file in all your modules, containing something like below:
     freeze.store.default.allowStoreCreation=true
     freeze.store.default.path=@project.basedir@/src/test/resources/archunit_store

Make sure filtering is enabled on test resources, and careful with the placeholder pattern (above, I use @*@, but maybe yours is different)

  1. In the "freezing rule" you've packaged to share and apply across modules, before building and executing your rule, you need to override the store path value :
    @Override
    public void execute(String packagePath, ScopePathProvider scopePathProvider, Collection<String> excludedPaths) {

        URL archUnitPropertiesUrl = getCurrentClassLoader(getClass()).getResource("archunit.properties");

        if(archUnitPropertiesUrl!=null) {

            Properties properties = new Properties();
            try (InputStream inputStream = archUnitPropertiesUrl.openStream()) {
                properties.load(inputStream);
            } catch (IOException e) {
                log.warn("Error reading ArchUnit properties from " + archUnitPropertiesUrl, e);
            }

            String pathToViolationsStore = properties.getProperty("freeze.store.default.path");

            if (pathToViolationsStore != null) {
                log.info("overriding violation store path to " + pathToViolationsStore);
                ArchConfiguration.get().setProperty("freeze.store.default.path", pathToViolationsStore);
            }
            else{
                log.warn("Could not find expected property, so using whatever value has been defined/overridden before");
            }
        }
        else{
            log.warn("Could not find expected property, so using whatever value has been defined/overridden before");
        }

        JavaClasses importedClasses = ArchUtils.importAllClassesInPackage(scopePathProvider.getMainClassesPath(),packagePath,excludedPaths);

        ArchRule rule = FreezingArchRule.freeze(
                // YOUR RULE HERE
              )
        );

        rule.check(importedClasses);

    }

It's probably not the cleanest solution, but it should do the trick : on the multi-module project where I tested, I now get a violation store created in each module.

Let me know if that fixes it for you, and/or if you have a cleaner solution.

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