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

Gradle Plugin test classes include production classes (potential regression) #1400

Closed
chemicL opened this issue Feb 3, 2023 · 4 comments
Closed
Assignees
Labels
Milestone

Comments

@chemicL
Copy link

chemicL commented Feb 3, 2023

In Project Reactor's debug agent, we're using ByteBuddy and have a gradle validation for proper source set instrumentation.

Starting from 1.12.21 our mock gradle setup started to fail.

Our test runs a mock gradle setup that validates that production instrumentation is applied, but the instrumentation does not happen for the test sources if not configured via appropriate ByteBuddy configurations (byteBuddy vs testByteBuddy).

https://github.com/reactor/reactor-core/blob/main/reactor-tools/src/buildPluginTest/java/reactor/tools/agent/ApplyingByteBuddyPluginGradleTest.java

The mock Gradle configuration is here: https://github.com/reactor/reactor-core/blob/main/reactor-tools/src/buildPluginTest/resources/mock-gradle/build.gradle

The class expected to be instrumented: https://github.com/reactor/reactor-core/blob/main/reactor-tools/src/buildPluginTest/resources/mock-gradle/src/main/java/demo/SomeClass.java

The class not expected to be instrumented: https://github.com/reactor/reactor-core/blob/main/reactor-tools/src/buildPluginTest/resources/mock-gradle/src/test/java/demo/SomeClassTest.java

Surprisingly, after running that configuration, the build output has both classes in the ../java/test/ output directory, as demonstrated by the screenshot:

image

I would expect the runtime to use the ../java/main/ class and ../java/test not to contain the SomeClass.class file. The main directory contains a properly instrumented class file.

Thanks in advance for looking into this. If there is now a different way to configure this and it's not a bug, then apologies and I'd welcome an answer how to achieve what we had before.

@raphw
Copy link
Owner

raphw commented Feb 4, 2023

I wonder if this has been an error that always existed but is caused by a different default, I am using Gradle @InputFiles as input to a task and it's those files that are considered. Maybe those input files contain production classes for test sources?

You can force the old behavior by setting discovery = Discovery.NONE in the byteBuddyTest configuration. Before, discovery was disabled by default but this was aligned with the Maven plugin,

@raphw raphw self-assigned this Feb 4, 2023
@raphw raphw modified the milestones: 1.12.0, 1.12.22 Feb 4, 2023
@raphw
Copy link
Owner

raphw commented Feb 4, 2023

Never mind. The plugin reuses the "raw" folder for the test classes. I have to create a different name for this folder which is based on the target name.

Also, this raises a question if the Maven defaults are meaningful for Gradle as the incremental compilation requires that Byte Buddy also applies the plugin if no transformations are configured for a goal.

@raphw
Copy link
Owner

raphw commented Feb 4, 2023

Could you build Byte Buddy from master and try if the issue is solved?

@chemicL
Copy link
Author

chemicL commented Feb 6, 2023

Thank you @raphw for the quick analysis and solution. I confirmed 1.12.23 does indeed work as expected with regards to our mock gradle setup. Great job 🥇

@chemicL chemicL closed this as completed Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants