milamberspace opened a new issue, #6724:
URL: https://github.com/apache/jmeter/issues/6724
## Context
#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.
- Related PR: #6718 (the fix that surfaced this).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
