FreezingArchRule breaks when executed from IDE and maven #923

mustaphazorgati · 2022-07-20T13:26:26Z

Environment:

JDK11

Expected behavior:
When the store files are generated, the tests should all pass independent of the test execution (here: IntelliJ or maven)

Actual behavior:

When the store files were generated through IntelliJ, the tests only pass when they are executed through IntelliJ. Executing the tests through maven will result in test failures.
When the store files were generated through maven, the tests only pass when they are executed through maven. Executing the tests through IntelliJ will result in test failures.

How to reproduce:

Check out this PR: TSK-1919: re-enabled architecture tests and fixed freeze mechanism Taskana/taskana#1971 where we've enabled our architecture tests (store files were generated through IntelliJ)
Execute the ArchitectureTest class through IntelliJ. All tests should pass.
Execute the ArchitectureTest class through maven. Some tests should fail.
Execution command (from project root directory):

#build dependencies
./mvnw clean install -pl :taskana-core-test -am -DskipTests
#execute tests from ArchitectureTest class
./mvnw clean verify -pl :taskana-core-test -Dtest="ArchitectureTest"

Further notes:

As of right now we've disabled the tests, since this behavior is blocking us.
You can find our test class here: https://github.com/Taskana/taskana/blob/master/lib/taskana-core-test/src/test/java/acceptance/ArchitectureTest.java
The tests packagesShouldBeFreeOfCyclicDependencies and classesShouldBeFreeOfCyclicDependencies will fail.
We've already set the following properties to an unreasonable high amount in order to make sure that all errors are detected.

cycles.maxNumberToDetect=100000
cycles.maxNumberOfDependenciesPerEdge=10000

During our initial debugging we've found out that the line numbers differ between the execution through maven and IntelliJ. Furthermore we've seen that IntelliJ and maven name lambda's differently, which is a second difference in the store files.

The text was updated successfully, but these errors were encountered:

codecholeric · 2022-07-24T17:01:48Z

Thanks for raising the issue! First, about the lambda parameter, can you please try version 1.0.0-rc1? Because these dependencies should be correctly dereferenced then and methods like lambda$xxx$123 should vanish...

Other than that typically this is a mismatch in classpath between the build tool and the IDE. But in this case I'm a little puzzled, because you describe that the line numbers are different? 🤔 Because that should not really be affected by the tool... Have you checked which line numbers actually make sense? I would guess only one version can report the correct line numbers then, right? Independently of that, even if the line numbers would change, this wouldn't lead the rule to fail, because FreezingArchRule should ignore changed line numbers (as that is a common case for frozen violations, that they are not really fixed, but just "moved around" within the file)

I can look into it a little more, my only problem is that I tried to check out that branch from your notes, but I can't even compile the project 🤔

./mvnw clean compile

...

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] warnings found and -Werror specified
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for pro.taskana:taskana-parent 5.2.2-SNAPSHOT:
[INFO] 
[INFO] pro.taskana:taskana-parent ......................... SUCCESS [  0.246 s]
[INFO] pro.taskana:taskana-common-parent .................. SUCCESS [  0.008 s]
[INFO] pro.taskana:taskana-common-logging ................. FAILURE [  0.430 s]
[INFO] pro.taskana:taskana-common-security ................ SKIPPED
...

The bizarre thing is that it doesn't even give me any details about what the error is. Do you have any hint what I have to do to get it to compile and why there are no error details in the Maven output? (I tried -e and -X, but it also didn't give me any further insights)

codecholeric · 2022-07-24T17:09:15Z

Nevermind, of course wrong Java version 🤦‍♂️ I thought it was set to 11, but it was actually set to 17. Still, would have been nice to get some other error details from Maven than just an empty message 🤪
Anyway, I'll try to look into it as soon as I find some time!

mustaphazorgati · 2022-07-25T09:51:13Z

Nevermind, of course wrong Java version man_facepalming I thought it was set to 11, but it was actually set to 17. Still, would have been nice to get some other error details from Maven than just an empty message zany_face

Yeah. Maven is sometimes funny and doesn't describe the error properly.

Anyway, I'll try to look into it as soon as I find some time!

Thank you very much! Can I assist you somehow? If so, then please let me know. I'll check out version 1.0.0-rc1 and see if that helps :)

codecholeric · 2022-07-25T09:58:56Z

Yes, checking out the difference if you use 1.0.0-rc1 would definitely be helpful 🙂

mustaphazorgati · 2022-07-25T10:07:22Z

I've just updated the branch and did some testing. Now the behavior is a little different. Unfortunately the tests still fail when the store files are generated through different ways (maven / IntelliJ).
What I have observed:
Now the delta contains only deletions. When the store files were generated through IntelliJ the execution through maven will delete some frozen violations and throw those violations as errors thus the test execution fails. This is the same observation when the store files are generated through maven and afterwards executed through IntelliJ.

codecholeric · 2022-08-26T13:49:14Z

I finally found some time to look into it, this is how it looked to me:

When I use 1.0.0-rc1 and delete the existing ViolationStore, then when I create a new store with IntelliJ Maven is afterwards also green and the store unchanged. And vice versa, if I create the store with Maven and then run the test with IntelliJ it's green and the store unchanged.
When I run 1.0.0-rc1 against the checked in ViolationStore, then it removes a couple of violations and reports them as new. I also noticed that for the checked in ViolationStore the lines of the violations don't seem to always be lexicographically ordered 🤔 I don't know how this would happen. Originally the ordering of the SliceRule was somewhat random, which made it hard to use with FreezingArchRule. But for that reason we have improved it over time and AFAIS the ordering was improved before the new format with indentation was introduced (which the checked in ViolationStore already uses). E.g.

2. Dependencies of Slice testapi.extensions\
    - Method <pro.taskana.testapi.extensions.TestContainerExtension.interceptTestClassConstructor(org.junit.jupiter.api.extension.InvocationInterceptor$Invocation, org.junit.jupiter.api.extension.ReflectiveInvocationContext, org.junit.jupiter.api.extension.ExtensionContext)> references class object <pro.taskana.testapi.CleanTaskanaContext> in (TestContainerExtension.java:41)\
    - Method <pro.taskana.testapi.extensions.TaskanaDependencyInjectionExtension.postProcessTestInstance(java.lang.Object, org.junit.jupiter.api.extension.ExtensionContext)> references class object <pro.taskana.testapi.TaskanaInject> in (TaskanaDependencyInjectionExtension.java:44)\
    - Method <pro.taskana.testapi.extensions.TestContainerExtension.interceptTestClassConstructor(org.junit.jupiter.api.extension.InvocationInterceptor$Invocation, org.junit.jupiter.api.extension.ReflectiveInvocationContext, org.junit.jupiter.api.extension.ExtensionContext)> calls method <pro.taskana.testapi.DockerContainerCreator.createDockerContainer(pro.taskana.common.internal.configuration.DB)> in (TestContainerExtension.java:46)\
    - Method <pro.taskana.testapi.extensions.TestContainerExtension.interceptTestClassConstructor(org.junit.jupiter.api.extension.InvocationInterceptor$Invocation, org.junit.jupiter.api.extension.ReflectiveInvocationContext, org.junit.jupiter.api.extension.ExtensionContext)> calls method <pro.taskana.testapi.DockerContainerCreator.createDataSource(org.testcontainers.containers.JdbcDatabaseContainer)> in (TestContainerExtension.java:50)\

With 1.0.0-rc1 the lambda$123 violations vanish. Normally that would keep a frozen rule green and just remove some lines where lambda$123 was reported. But for SliceRule this is a little bit more tricky, because there it actually saves multiline violations, i.e. one Cycle detected: Slice a -> \... multiline violation will be one entry in the store. Thus, if a line from this multiline string vanishes, it will be seen as a different line / violation, i.e. the old one will be removed and that one reported. So it might be necessary to refreeze the SliceRule violations after upgrading ArchUnit.

Can you check, if you have the same problem if you really refreeze the violations with 1.0.0-rc1? I.e. that frozen from one environment (Maven/IDE) they are failing on the other environment? You should be able to easily do this e.g. with the property freeze.refreeze=true

mustaphazorgati · 2022-09-10T11:41:30Z

Hi @codecholeric,
sorry for the late response. The last two weeks have been rather busy.

When I use 1.0.0-rc1 and delete the existing ViolationStore, then when I create a new store with IntelliJ Maven is afterwards also green and the store unchanged. And vice versa, if I create the store with Maven and then run the test with IntelliJ it's green and the store unchanged.

Unfortunately I cannot verify this. I've updated the branch, removed the existing ViolationStore and upgraded Archunit to version 1.0.0-rc1. A single rule gets removed and thrown as an error. I can just fix this rule and would be happy, but there seems to be something off and I bet you prefer to find what's causing the issue.
You can recreate the scenario by creating a new ViolationStore through IntelliJ / Maven and then execute the tests afterwards from the other.

When I run 1.0.0-rc1 against the checked in ViolationStore, then it removes a couple of violations and reports them as new. I also noticed that for the checked in ViolationStore the lines of the violations don't seem to always be lexicographically ordered thinking I don't know how this would happen. Originally the ordering of the SliceRule was somewhat random, which made it hard to use with FreezingArchRule. But for that reason we have improved it over time and AFAIS the ordering was improved before the new format with indentation was introduced (which the checked in ViolationStore already uses). E.g.

Thank you for this explanation. Indeed the one "failing" violation gets removed from a frozen SliceRule.

With 1.0.0-rc1 the lambda$123 violations vanish. Normally that would keep a frozen rule green and just remove some lines where lambda$123 was reported. But for SliceRule this is a little bit more tricky, because there it actually saves multiline violations, i.e. one Cycle detected: Slice a -> ... multiline violation will be one entry in the store. Thus, if a line from this multiline string vanishes, it will be seen as a different line / violation, i.e. the old one will be removed and that one reported. So it might be necessary to refreeze the SliceRule violations after upgrading ArchUnit.

Yep, this issue is gone.

Can you check, if you have the same problem if you really refreeze the violations with 1.0.0-rc1? I.e. that frozen from one environment (Maven/IDE) they are failing on the other environment? You should be able to easily do this e.g. with the property freeze.refreeze=true

Unfortunately I still have an issue with a single rule. As I mentioned above, I can just fix the rule and would be happy.
Do you want to have another look at this or should we call this done?

Quick question: Do you have any gut feeling / ETA when version 1.0.0 will be released? I am not sure if the team would welcome a release candidate as a dependency

codecholeric · 2022-09-11T15:26:05Z

My problem is that I can't reproduce it anymore 🤔 I pulled your latest changes, then recompiled everything, ran the test from IntelliJ, the store was created, ran it from Maven -> success. Deleted the store, recreated it from Maven, ran the test in IntelliJ -> success.
So there either must be some random factor, or it depends on the platform. What OS are you running the tests on? Windows by any chance? 😉 (just asking, cause I had some problems in the past; I'm running the tests on Linux...)
Could you maybe create the store from your IDE, safe copy it somewhere, create it from Maven as well, and then zip both stores and attach them to a comment here? Because otherwise I wouldn't know how to dig deeper into this 🤔
Regarding 1.0.0, this is the last issue I'm looking into. If we're sure it's not a blocker I can release 1.0.0

mustaphazorgati · 2022-09-12T09:31:31Z

Hey @codecholeric,

My problem is that I can't reproduce it anymore thinking I pulled your latest changes, then recompiled everything, ran the test from IntelliJ, the store was created, ran it from Maven -> success. Deleted the store, recreated it from Maven, ran the test in IntelliJ -> success.

I've provided a video where I showcase what I did. Hopefully this helps you.
https://drive.google.com/file/d/1KRMzIp7ipZYZxynLzKX8HZ1XMx7YTafJ/view?usp=sharing

So there either must be some random factor, or it depends on the platform. What OS are you running the tests on? Windows by any chance? wink (just asking, cause I had some problems in the past; I'm running the tests on Linux...)

I am using linux myself.

[mustapha@NT220403 taskana]$ uname -r
5.19.7-arch1-1
[mustapha@NT220403 taskana]$ cat /etc/os-release 
NAME="Arch Linux"
PRETTY_NAME="Arch Linux"
ID=arch
BUILD_ID=rolling
ANSI_COLOR="38;2;23;147;209"
HOME_URL="https://archlinux.org/"
DOCUMENTATION_URL="https://wiki.archlinux.org/"
SUPPORT_URL="https://bbs.archlinux.org/"
BUG_REPORT_URL="https://bugs.archlinux.org/"
LOGO=archlinux-logo

Could you maybe create the store from your IDE, safe copy it somewhere, create it from Maven as well, and then zip both stores and attach them to a comment here? Because otherwise I wouldn't know how to dig deeper into this thinking

sure thing :) In the video you can see how I created these files.
archunit_store_maven.zip
archunit_store_intellij.zip
output_intellij.txt
output_maven.txt

Regarding 1.0.0, this is the last issue I'm looking into. If we're sure it's not a blocker I can release 1.0.0

Alright. I don't want to be the blocking factor. Therefore I will try to reply as fast as I can :)

codecholeric · 2022-09-13T06:44:19Z

Thanks a lot for the extensive info 😍 That helps a lot! On the first sight it looks like each environment detects another cycle within pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer and inner classes 🤯 I'll try to understand what exactly goes on. Weird though, that we both run it on a quite similar environment, yet it behaves that differently 😬 (because from your video I can see that I did almost the exact same things as you and for me the test doesn't fail)

mustaphazorgati · 2022-09-13T07:02:08Z

Thanks a lot for the extensive info heart_eyes That helps a lot!

You're welcome!

On the first sight it looks like each environment detects another cycle within pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer and inner classes exploding_head I'll try to understand what exactly goes on.

That's interesting. Hopefully you can find the weird behaving spot.

Weird though, that we both run it on a quite similar environment, yet it behaves that differently grimacing (because from your video I can see that I did almost the exact same things as you and for me the test doesn't fail)

To be honest I didn't expect that.

If I can assist you somehow please let me know :)

Calls to lambda methods `lambda$abc$123` seem to have unique origins with current JDK versions. I.e. even two identical consecutive lambda invocations will lead to two separate synthetic methods. Unfortunately, this is not the case for synthetic `access$123` methods (to circumvent private modifiers), which lead to a bug where multiple origins of such a call would override each other. This lead e.g. to the behavior in #923, where the application of AspectJ introduced an `access$123` method that was called from multiple places. But, only one such call would randomly be created during the import, which lead to an unpredictable behavior of which cycles between outer and inner classes would be detected. We now record multiple access records for any synthetic call and trace back all those records instead of just the last one. This way all such synthetic accesses should be imported correctly now. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>

codecholeric · 2022-09-16T17:21:29Z

Okay, this was quite the rabbit hole 😂 The first thing I noticed was that while some cycles between these classes were missing, this would not happen anymore if I would disable the AspectJ weaving. Looking into the bytecode I noticed that applying AspectJ would make the call to TaskanaConfigurationInitializer.parseProperty(..) invisible in the bytecode of the classes. This is because AspectJ would weave in some additional synthetic access methods TaskanaConfigurationInitializer.access$123 which would then call parseProperty.
So the question at this point was only, why was one cycle always detected if this call was missing, or more precisely, why was only one cycle detected, since normally ArchUnit would be supposed to transparently replace those calls to access$123 with the call users would expect from the source code (compare #889).
Turns out that 1.0.0-rc1 actually contains a bug there 🙈 So, I'm really glad that this issue brought this to surface. Basically, the implementation to remove those access$123 methods was the same as for lambda$xyz$123 methods, but for the latter the origins of the calls would be unique. But, for access$123 they actually overrode each other, which lead to a non-deterministic behavior which access would be imported in the end (but seemingly deterministic enough so that it would be the same as long as the system and the environment would not change). You can see more details and (hopefully) a fix here (#957).
In any case, I also published the version with the fix as 1.0.0-SNAPSHOT, so maybe you can test this, if it solves your problem? It should now detect all 6 cycles for all 6 inner classes of TaskanaConfigurationInitializer and thus not have different frozen cycles on different environments anymore. You have to add the snapshot repository to test it:

<repositories>
    <repository>
      <id>sonatype-snapshots</id>
      <url>https://oss.sonatype.org/content/repositories/snapshots</url>
    </repository>
  </repositories>

Can you let me know if this now behaves reasonable for you?

mustaphazorgati · 2022-09-16T20:59:39Z

Okay, this was quite the rabbit hole joy The first thing I noticed was that while some cycles between these classes were missing, this would not happen anymore if I would disable the AspectJ weaving. Looking into the bytecode I noticed that applying AspectJ would make the call to TaskanaConfigurationInitializer.parseProperty(..) invisible in the bytecode of the classes. This is because AspectJ would weave in some additional synthetic access methods TaskanaConfigurationInitializer.access$123 which would then call parseProperty.
So the question at this point was only, why was one cycle always detected if this call was missing, or more precisely, why was only one cycle detected, since normally ArchUnit would be supposed to transparently replace those calls to access$123 with the call users would expect from the source code (compare #889).
Turns out that 1.0.0-rc1 actually contains a bug there see_no_evil So, I'm really glad that this issue brought this to surface. Basically, the implementation to remove those access$123 methods was the same as for lambda$xyz$123 methods, but for the latter the origins of the calls would be unique. But, for access$123 they actually overrode each other, which lead to a non-deterministic behavior which access would be imported in the end (but seemingly deterministic enough so that it would be the same as long as the system and the environment would not change). You can see more details and (hopefully) a fix here (#957).

Now that you mention it, it clicks. I have disabled the AspectJ weaving within IntelliJ a while ago. I've totally forgot to mention that THAT is the difference between the execution of maven and IntelliJ. This could have probably saved some time on your end... Sorry 😞

Nevertheless, I've tested 1.0.0-SNAPSHOT intensively:

Compile all modules with AspectJ weaving ON through IntelliJ ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
Compile all modules with AspectJ weaving OFF through IntelliJ ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
Compile all modules with AspectJ weaving ON through maven ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
Compile all modules with AspectJ weaving OFF through maven ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through IntelliJ with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️
- Creating the store through maven with aspectj-maven-plugin NOT RUNNING ✔️
  
  Execution through aspectj-maven-plugin test result
  
  IntelliJ running ✔️
  
  IntelliJ not running ✔️
  
  maven running ✔️
  
  maven not running ✔️

Everything seems to work just fine :) 😄
Thank you very much for your kind and professional support 🥇 ❤️ - from my perspective this issue is fixed. Can't wait for the release 🥳

mustaphazorgati · 2022-09-16T23:35:38Z

I am very sorry, but I have to reopen this issue. My previous testing method is flawed and did not target the "enemy": AspectJ.
What I've tested is that maven and IntelliJ behave the same, which they do; and did from the beginning.

What I did not test is the behaviour of AspectJ. And this still seems to be broken.

Issue: The store content is not the same when the compilation IS weaved with AspectJ and IS NOT weaved with AspectJ. This results to test failures when executing the architecture tests on a compilation which is not weaved with AspectJ when the store was created with a weaved compilation and vice verca.

Steps to reproduce:

Scenario 1:

disable AspectJ
compile the project
Create the store
enable AspectJ
compile the project
execute architecture tests

Commands to reproduce scenario 1:

# Check out PR (https://github.com/Taskana/taskana/pull/1971)
gh pr checkout 1971
# Clear TASKANA artifacts from local maven repository
rm -rf ~/.m2/repository/pro/taskana/*
# Remove existing store
rm -rf lib/taskana-core-test/archunit_store
# Compile TASKANA without AspectJ
./mvnw clean install -pl :taskana-core-test -am -DskipTests -DskipAspectJ
# Build store 
./mvnw clean verify -pl :taskana-core-test -Dtest="ArchitectureTest"
# OPTIONAL: Create zip of store
zip -r archunit_store_compiled_without_aspectj.zip lib/taskana-core-test/archunit_store
# Compile TASKANA with AspectJ
./mvnw clean install -pl :taskana-core-test -am -DskipTests
# Execute architecture tests with AspectJ
./mvnw clean verify -pl :taskana-core-test -Dtest="ArchitectureTest" | tee test_execution_with_aspectj_using_store_without_aspectj.txt

Store files: archunit_store_compiled_without_aspectj.zip
Test execution: test_execution_with_aspectj_using_store_without_aspectj.zip (I had to zip the txt file due to upload size limitations)

Scenario 2:

enable AspectJ
compile the project
Create the store
disable AspectJ
compile the project
execute architecture tests

Commands to reproduce scenario 2:

# Check out PR (https://github.com/Taskana/taskana/pull/1971)
gh pr checkout 1971
# Clear TASKANA artifacts from local maven repository
rm -rf ~/.m2/repository/pro/taskana/*
# Remove existing store
rm -rf lib/taskana-core-test/archunit_store
# Compile TASKANA with AspectJ
./mvnw clean install -pl :taskana-core-test -am -DskipTests
# Build store 
./mvnw clean verify -pl :taskana-core-test -Dtest="ArchitectureTest"
# OPTIONAL: Create zip of store
zip -r archunit_store_compiled_with_aspectj.zip lib/taskana-core-test/archunit_store
# Compile TASKANA without AspectJ
./mvnw clean install -pl :taskana-core-test -am -DskipTests -DskipAspectJ
# Execute architecture tests without AspectJ
./mvnw clean verify -pl :taskana-core-test -Dtest="ArchitectureTest" -DskipAspectJ | tee test_execution_without_aspectj_using_store_with_aspectj.txt

Store files: archunit_store_compiled_with_aspectj.zip
Test execution: test_execution_without_aspectj_using_store_with_aspectj.zip
(I had to zip the txt file due to upload size limitations)

Also I've provided a video where I showcase both scenarios. That video also shows how those files were created :) https://drive.google.com/file/d/1zmSe2pfjLeq3TevsPPToeWiUU3KwcwVw/view?usp=sharing

Now the question: Should ArchUnit behave differently when AspectJ is enabled/disabled? Is this even a valid (supported) usecase for ArchUnit? If yes: We should find a solution. If not: We can close this issue.

codecholeric · 2022-09-17T10:29:55Z

The problem is, I think asking that it behaves the same with and without AspectJ is impossible. Because AspectJ just fundamentally changes the bytecode. And the bytecode is the source of truth for ArchUnit. ArchUnit has no way to know what is from AspectJ and what not. Compare for example the output for ListPropertyParser without AspectJ (which makes sense and matches the source code)

- Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.initialize(java.util.Properties, java.lang.String, java.lang.reflect.Field, pro.taskana.common.internal.configuration.TaskanaProperty)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.splitStringAndTrimElements(java.lang.String, java.lang.String)> in (TaskanaConfigurationInitializer.java:184)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.initialize(java.util.Properties, java.lang.String, java.lang.reflect.Field, pro.taskana.common.internal.configuration.TaskanaProperty)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.createCustomHolidayFromPropsEntry(java.lang.String)> in (TaskanaConfigurationInitializer.java:188)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.initialize(java.util.Properties, java.lang.String, java.lang.reflect.Field, pro.taskana.common.internal.configuration.TaskanaProperty)> gets field <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.LOGGER> in (TaskanaConfigurationInitializer.java:190)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.initialize(java.util.Properties, java.lang.String, java.lang.reflect.Field, pro.taskana.common.internal.configuration.TaskanaProperty)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.parseProperty(java.util.Properties, java.lang.String, pro.taskana.common.internal.util.CheckedFunction)> in (TaskanaConfigurationInitializer.java:196)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.initialize(java.util.Properties, java.lang.String, java.lang.reflect.Field, pro.taskana.common.internal.configuration.TaskanaProperty)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.splitStringAndTrimElements(java.lang.String, java.lang.String, java.util.function.UnaryOperator)> in (TaskanaConfigurationInitializer.java:200)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.initialize(java.util.Properties, java.lang.String, java.lang.reflect.Field, pro.taskana.common.internal.configuration.TaskanaProperty)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.parseProperty(java.util.Properties, java.lang.String, pro.taskana.common.internal.util.CheckedFunction)> in (TaskanaConfigurationInitializer.java:201)

versus the output after AspectJ has rewritten the bytecode

- Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.lambda$1(java.lang.String)> gets field <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.LOGGER> in (TaskanaConfigurationInitializer.java:38)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.initialize(java.util.Properties, java.lang.String, java.lang.reflect.Field, pro.taskana.common.internal.configuration.TaskanaProperty)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.parseProperty(java.util.Properties, java.lang.String, pro.taskana.common.internal.util.CheckedFunction)> in (TaskanaConfigurationInitializer.java:152)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.lambda$0(java.lang.String, java.lang.String)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.splitStringAndTrimElements(java.lang.String, java.lang.String)> in (TaskanaConfigurationInitializer.java:184)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.lambda$1(java.lang.String)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.createCustomHolidayFromPropsEntry(java.lang.String)> in (TaskanaConfigurationInitializer.java:188)\
    - Method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer$ListPropertyParser.lambda$3(java.lang.String)> calls method <pro.taskana.common.internal.configuration.TaskanaConfigurationInitializer.splitStringAndTrimElements(java.lang.String, java.lang.String, java.util.function.UnaryOperator)> in (TaskanaConfigurationInitializer.java:200)

It's just fundamentally different, the calls come from other places and AspectJ also weaves in some "weird" methods (lambda$1 and co. are no standard conventions for lambda methods as the JDK writes them).

So, to wrap it up I think ArchUnit tests should run on unmanipulated bytecode. Because that bytecode still matches the source code well. In the end, AspectJ could also weave in a lot of additional calls that are not visible in the source code 🤷 So, it can never be the same with or without AspectJ independently of the Aspects that are woven in.

mustaphazorgati · 2022-09-17T11:23:58Z

I can totally understand your reasoning. Thanks for the explanation. :)

…957) Calls to lambda methods `lambda$abc$123` seem to have unique origins with current JDK versions. I.e. even two identical consecutive lambda invocations will lead to two separate synthetic methods. Unfortunately, this is not the case for synthetic `access$123` methods (to circumvent private modifiers), which lead to a bug where multiple origins of such a call would override each other. This lead e.g. to the behavior in #923, where the application of AspectJ introduced an `access$123` method that was called from multiple places. But, only one such call would randomly be created during the import, which lead to an unpredictable behavior of which cycles between outer and inner classes would be detected. We now record multiple access records for any synthetic call and trace back all those records instead of just the last one. This way all such synthetic accesses should be imported correctly now.

codecholeric mentioned this issue Sep 16, 2022

fix bug where origins of synthetic method calls override each other #957

Merged

mustaphazorgati closed this as completed Sep 16, 2022

mustaphazorgati reopened this Sep 16, 2022

mustaphazorgati closed this as completed Sep 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FreezingArchRule breaks when executed from IDE and maven #923

FreezingArchRule breaks when executed from IDE and maven #923

mustaphazorgati commented Jul 20, 2022 •

edited

Loading

codecholeric commented Jul 24, 2022

codecholeric commented Jul 24, 2022

mustaphazorgati commented Jul 25, 2022 •

edited

Loading

codecholeric commented Jul 25, 2022

mustaphazorgati commented Jul 25, 2022 •

edited

Loading

codecholeric commented Aug 26, 2022 •

edited

Loading

mustaphazorgati commented Sep 10, 2022

codecholeric commented Sep 11, 2022 •

edited

Loading

mustaphazorgati commented Sep 12, 2022

codecholeric commented Sep 13, 2022

mustaphazorgati commented Sep 13, 2022 •

edited

Loading

codecholeric commented Sep 16, 2022

mustaphazorgati commented Sep 16, 2022

mustaphazorgati commented Sep 16, 2022 •

edited

Loading

codecholeric commented Sep 17, 2022

mustaphazorgati commented Sep 17, 2022

FreezingArchRule breaks when executed from IDE and maven #923

FreezingArchRule breaks when executed from IDE and maven #923

Comments

mustaphazorgati commented Jul 20, 2022 • edited Loading

codecholeric commented Jul 24, 2022

codecholeric commented Jul 24, 2022

mustaphazorgati commented Jul 25, 2022 • edited Loading

codecholeric commented Jul 25, 2022

mustaphazorgati commented Jul 25, 2022 • edited Loading

codecholeric commented Aug 26, 2022 • edited Loading

mustaphazorgati commented Sep 10, 2022

codecholeric commented Sep 11, 2022 • edited Loading

mustaphazorgati commented Sep 12, 2022

codecholeric commented Sep 13, 2022

mustaphazorgati commented Sep 13, 2022 • edited Loading

codecholeric commented Sep 16, 2022

mustaphazorgati commented Sep 16, 2022

mustaphazorgati commented Sep 16, 2022 • edited Loading

codecholeric commented Sep 17, 2022

mustaphazorgati commented Sep 17, 2022

mustaphazorgati commented Jul 20, 2022 •

edited

Loading

mustaphazorgati commented Jul 25, 2022 •

edited

Loading

mustaphazorgati commented Jul 25, 2022 •

edited

Loading

codecholeric commented Aug 26, 2022 •

edited

Loading

codecholeric commented Sep 11, 2022 •

edited

Loading

mustaphazorgati commented Sep 13, 2022 •

edited

Loading

mustaphazorgati commented Sep 16, 2022 •

edited

Loading