Skip to content

Commit

Permalink
Adjust OSGiWeavingAdaptorTest to upstream AspectJ change
Browse files Browse the repository at this point in the history
  • Loading branch information
kriegaex committed Apr 8, 2024
1 parent cd75d99 commit 84c7931
Showing 1 changed file with 6 additions and 2 deletions.
Expand Up @@ -100,7 +100,11 @@ public void acceptClass(String name, byte[] originalBytes, byte[] weavedBytes) {
actualResult.containsKey("my.foo.MyBaseClass$AjcClosure1"));
assertTrue("my.foo.MyBaseClass$AjcClosure3 is not contained in " + actualContents,
actualResult.containsKey("my.foo.MyBaseClass$AjcClosure3"));
assertEquals("Expected size 4 of result " + actualContents,
4, actualResult.size());
assertTrue("my.foo.MyBaseClass is not contained in " + actualContents,
actualResult.containsKey("my.foo.MyBaseClass"));
assertFalse("my.foo.MyClass is contained in " + actualContents,
actualResult.containsKey("my.foo.MyClass"));
assertEquals("Expected size 5 of result " + actualContents,
5, actualResult.size());
}
}

2 comments on commit 84c7931

@xpomul
Copy link
Contributor

@xpomul xpomul commented on 84c7931 Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kriegaex commenting here directly (I don't see the issue or PR to which this commit belongs.

I am fine with the change. The important line with respect to the fix addressed in this test case is

if (!name.equals(className) && name.equals(unwovenClass.getClassName())) {

which makes sure that we do not store the woven bytes under a wrong name (which is derived from the map entry's key) in the cache. As long as that remains true, I don't see a problem with upstream changes.

Just a tiny nitpicky issue: Line 84

// - the result should not include the non-generated class, only the four generated ones

still mentions four expected entries, but these are now 5. Maybe just remove the concrete number there to not cause any followup confusion.

@kriegaex
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the comment in 56c2333, thanks for your feedback.

Please sign in to comment.