MCP-57 Expand on integration tests against SQC - #429
Conversation
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
627c6b7 to
5e33aed
Compare
5e33aed to
af18e4e
Compare
6a2ec34 to
f13e0fc
Compare
7cbe289 to
7943665
Compare
cfa02b2 to
e478874
Compare
4b75628 to
20eec0d
Compare
20eec0d to
0d9aa42
Compare
damien-urruty-sonarsource
left a comment
damien-urruty-sonarsource
left a comment
There was a problem hiding this comment.
Looks quite good to me, I left a couple of remarks/questions
|
|
||
| - name: Install Maven | ||
| if: matrix.sonarcloud == true | ||
| run: sudo apt-get update && sudo apt-get install -y maven |
There was a problem hiding this comment.
Shouldn't we use Mise instead?
| version: 2026.4.20 | ||
|
|
||
| - name: Install Maven | ||
| run: sudo apt-get update && sudo apt-get install -y maven |
| ARTIFACTORY_URL: ${{ steps.secrets-sc.outputs.vault && fromJSON(steps.secrets-sc.outputs.vault).ARTIFACTORY_URL || '' }} | ||
| ARTIFACTORY_PRIVATE_USERNAME: ${{ steps.secrets-sc.outputs.vault && fromJSON(steps.secrets-sc.outputs.vault).ARTIFACTORY_USER || '' }} | ||
| ARTIFACTORY_PRIVATE_PASSWORD: ${{ steps.secrets-sc.outputs.vault && fromJSON(steps.secrets-sc.outputs.vault).ARTIFACTORY_ACCESS_TOKEN || '' }} |
There was a problem hiding this comment.
This syntax looks different than what we usually have, why do we need the || '' at the end ? We should always expect the variable to be present
| if [ -z "${SONARCLOUD_IT_TOKEN}" ]; then | ||
| echo "::warning::SONARCLOUD_IT_TOKEN not available (e.g. fork PR); skipping SonarQube Cloud staging ITS" | ||
| exit 0 |
There was a problem hiding this comment.
We should fail harder than that IMO. It's somehow related to my previous comment, I don't know why we fallback to empty string in some cases
EDIT: I see your point about forks, but I think we shouldn't care too much about that
| on: | ||
| schedule: | ||
| # Weekly on Monday at 06:00 UTC | ||
| - cron: '0 6 * * 1' |
There was a problem hiding this comment.
I assume this is because there is low activity in the project?
There was a problem hiding this comment.
Yes, I wanted to run the tests on a weekly basis (or maybe daily?) to check any sudden change on SQC - given that we may not always have activity on this repo
| .contains("## Context Augmentation") | ||
| .contains("All tools loaded: 34 tools after category filtering"); | ||
| .contains("Advanced analysis mode enabled") | ||
| .contains("All tools loaded: 40 tools after category filtering"); |
There was a problem hiding this comment.
Do you know why we have a different number of tools loaded? It seems strange to have different expected things for a PR that only adds tests
| var result = mcpClient.callTool(ProjectStatusTool.TOOL_NAME, Map.of("projectKey", fixture.projectKey())); | ||
|
|
||
| var content = structuredContent(result); | ||
| assertThat(content.get("status")).isIn("OK", "WARN", "ERROR", "NONE"); |
There was a problem hiding this comment.
This is also not very intentional. I think we should check what we expect
|
|
||
| assertResultEquals(result, """ | ||
| { | ||
| "webhooks" : [ ] |
There was a problem hiding this comment.
Should we have a least one webhook to check the format?
| assertThat(content).containsKey("enterprises"); | ||
| assertThat(content.get("enterprises")).isInstanceOf(java.util.List.class); |
There was a problem hiding this comment.
Should we check the output more precisely?
| echo "::warning::SONARCLOUD_IT_TOKEN not available (e.g. fork PR); skipping SonarQube Cloud staging ITS" | ||
| exit 0 | ||
| fi | ||
| ./gradlew :its:sonarCloudIntegrationTest --info |
There was a problem hiding this comment.
Maybe the check for SONARCLOUD_IT_TOKEN should be part of sonarCloudIntegrationTest? So it also fails early and explicitly locally
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
CI failed: The build is failing due to a compilation error in the integration tests module caused by a missing symbol, alongside intermittent failures in existing integration tests.OverviewThe CI build is failing primarily due to a compilation error in the FailuresCompilation Error in
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
2 participants
Summary by Gitar
sonarCloudIntegrationTesttask andSonarCloudStagingHarnessfor in-process MCP client testing against real SonarQube Cloud staging.sample-javaMaven project to ensure consistent analysis state.*SonarCloudIT) to validate staging parity for every MCP tool.ProxiedServerITestto reflect increased tool count (34 to 40).sonarcloud-integration.ymlweekly workflow and updatedbuild.ymlto run staging tests in parallel via matrix.BlockingQueueOutputStream(b & 0xFF) to prevent data corruption.This will update automatically on new commits.