Split out camel experimental assertions #14433
Conversation
buildscripts/checkstyle.xml
Outdated
| <module name="RegexpSinglelineJava"> | ||
| <property name="format" | ||
| value="(?<!import static io.opentelemetry.sdk.testing.assertj.)OpenTelemetryAssertions\."/> | ||
| value="(?<!import (?:static )?io.opentelemetry.sdk.testing.assertj.)OpenTelemetryAssertions\.(?![A-Z]\w*[;\s]*)"/> |
There was a problem hiding this comment.
The regex is too strict and catching inner classes, which in this case can't be statically imported. I updated the pattern to exclude both regular and static imports from the package, and to allow uppercase class names while still catching lowercase method calls that should be statically imported.
There was a problem hiding this comment.
that is quite hard to understand - can we make it 2 assertions?
| <module name="RegexpSinglelineJava"> | ||
| <property name="format" | ||
| value="(?<!import static io.opentelemetry.sdk.testing.assertj.)OpenTelemetryAssertions\."/> | ||
| value="^(?!.*import).*OpenTelemetryAssertions\.[a-z]"/> |
There was a problem hiding this comment.
why do you need a negative lookahead here? would it work without?
There was a problem hiding this comment.
without the negative lookback it catches lines like import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
[ant:checkstyle] [WARN] /Users/jaydeluca/code/projects/opentelemetry-java-instrumentation/instrumentation/ca
mel-2.20/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachecamel/ExperimentalTest.jav
a:8: Please statically import methods from OpenTelemetryAssertions [RegexpSinglelineJava]
There was a problem hiding this comment.
OK, got it now - maybe it's worth to add a comment explaining that
There was a problem hiding this comment.
i added a comment here, do you have a suggestion on how it could be more clear? I tried to explain it with the
but allow inner classes (uppercase) and imports
| <module name="RegexpSinglelineJava"> | ||
| <property name="format" | ||
| value="(?<!import static io.opentelemetry.sdk.testing.assertj.)OpenTelemetryAssertions\."/> | ||
| value="^(?!.*import).*OpenTelemetryAssertions\.[a-z]"/> |
There was a problem hiding this comment.
| value="^(?!.*import).*OpenTelemetryAssertions\.[a-z]"/> | |
| <!-- negative lookahead for "import", because imports are handled by the rule below --> | |
| value="^(?!.*import).*OpenTelemetryAssertions\.[a-z]"/> |
There was a problem hiding this comment.
the rule below is only for assertThat methods, but I pushed an update that calls out the lookahead being to exclude imports
Related to #14128, part of #13468
Created a separate test suite for experimental attributes and enabled telemetry collection.
the checkstyle task was failing with:
The regex is too strict and catching inner classes, which in this case can't be statically imported. I updated the pattern to exclude both regular and static imports from the package, and to allow uppercase class names while still catching lowercase method calls that should be statically imported.