You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#6718 fixed ~90 occurrences of SonarQube rule java:S3415 ("assertion arguments passed in the wrong order" — assertEquals(actual, expected) instead of assertEquals(expected, actual)) across the test suite.
These accumulated because SonarCloud analysis currently runs out-of-band — a nightly report to https://sonarcloud.io/dashboard?id=JMeter via the org.sonarqube Gradle plugin — not as a per-PR check. So nothing flags a new violation when a PR introduces one.
Opening this per the discussion on #6718 (@vlsi asked whether we can add something to avoid re-introducing this type of error).
Options (rough order of effort)
1. Add SonarCloud PR analysis to the GitHub Actions CI (preferred).
The org.sonarqube plugin is already wired (org apache, project JMeter); the missing piece is a sonar step on the PR workflow plus SonarCloud's "Clean as You Code" quality gate, which fails a PR that introduces a new violation. This directly targets S3415 (and every other Sonar rule) at review time. S3415 is heuristic (name-based — it fires when a literal/constant sits in the "actual" slot), so it catches the common cases, not 100%.
2. Error Prone ArgumentSelectionDefectChecker.
Error Prone already runs in CI (the "Error Prone (JDK 21)" job), so this is the cheapest to trial. It can flag swapped variables whose names don't match the expected / actual parameters, but is unreliable for literals. Evaluate signal vs. noise before promoting it to an error.
3. AssertJ migration (largest, most durable). assertThat(actual).isEqualTo(expected) removes the ambiguity by construction — there's no "which arg is expected?" left to get wrong. A big, separate effort, but it eliminates the whole class of bug instead of policing it.
Notes
SonarCloud PR analysis needs a SONAR_TOKEN (org apache) available to the workflow — coordinate with ASF Infra / the existing nightly setup, and mind that PR analysis from forks needs a pull_request_target-style flow or the token exposed safely.
Context
#6718 fixed ~90 occurrences of SonarQube rule
java:S3415("assertion arguments passed in the wrong order" —assertEquals(actual, expected)instead ofassertEquals(expected, actual)) across the test suite.These accumulated because SonarCloud analysis currently runs out-of-band — a nightly report to https://sonarcloud.io/dashboard?id=JMeter via the
org.sonarqubeGradle plugin — not as a per-PR check. So nothing flags a new violation when a PR introduces one.Opening this per the discussion on #6718 (@vlsi asked whether we can add something to avoid re-introducing this type of error).
Options (rough order of effort)
1. Add SonarCloud PR analysis to the GitHub Actions CI (preferred).
The
org.sonarqubeplugin is already wired (orgapache, projectJMeter); the missing piece is asonarstep on the PR workflow plus SonarCloud's "Clean as You Code" quality gate, which fails a PR that introduces a new violation. This directly targetsS3415(and every other Sonar rule) at review time.S3415is heuristic (name-based — it fires when a literal/constant sits in the "actual" slot), so it catches the common cases, not 100%.2. Error Prone
ArgumentSelectionDefectChecker.Error Prone already runs in CI (the "Error Prone (JDK 21)" job), so this is the cheapest to trial. It can flag swapped variables whose names don't match the
expected/actualparameters, but is unreliable for literals. Evaluate signal vs. noise before promoting it to an error.3. AssertJ migration (largest, most durable).
assertThat(actual).isEqualTo(expected)removes the ambiguity by construction — there's no "which arg is expected?" left to get wrong. A big, separate effort, but it eliminates the whole class of bug instead of policing it.Notes
SONAR_TOKEN(orgapache) available to the workflow — coordinate with ASF Infra / the existing nightly setup, and mind that PR analysis from forks needs apull_request_target-style flow or the token exposed safely.