Skip to content

[DO NOT MERGE] Validate E2E tests on VS 17.0 (ci-ami-images#520) and VS 18.12 - #6871

Draft
sophio-japharidze-sonarsource wants to merge 2 commits into
codex/slvs-3097-connected-smoke-spikefrom
codex/slvs-3097-e2e-vs17-vs18-matrix
Draft

sophio-japharidze-sonarsource wants to merge 2 commits into
codex/slvs-3097-connected-smoke-spikefrom
codex/slvs-3097-e2e-vs17-vs18-matrix

Conversation

@sophio-japharidze-sonarsource

@sophio-japharidze-sonarsource sophio-japharidze-sonarsource commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Do not merge. This PR only validates the VS 2022 17.0 minimum-compatibility image from ci-ami-images #520. Close it once the image is validated.

It is stacked on #6861, so it runs both E2E tests (standalone and connected mode) against:

Target Runner VS Community
Latest warp-custom-sonarlint-visualstudio (promoted master image) 18.12.12405.243
Earliest supported warp-custom-pr-520-sonarlint-visualstudio-vs17-0-0-v20261112124916 (#520 PR image) 17.0.31903.59

The VS 18.12 jobs check that the harness change below does not break the tests that already pass.

Commits

  1. Select the E2E target IDE by its expected major version (meant to be kept). The harness only accepted VS 18 instances (StartsWith("18.")). It now takes the major version from E2E_EXPECTED_VS_VERSION. This should move into SLVS-3097 Run extension E2E test on master and on PRs that change it #6854 or SLVS-3097 Add connected-mode extension E2E test #6861 before VS 17 coverage lands.
  2. [DO NOT MERGE] Run both E2E tests on VS 17.0 … and VS 18.12 (temporary). A fail-fast: false matrix on both jobs in extension-e2e.yml. The 17.0 runner exists only while Changed conflicts manager to only run in legacy connected mode #520 is open.

The sample is an old-style (non-SDK) project, so VS 17.0 can load it without the .NET SDK integration that the minimum image leaves out.

🤖 Generated with Claude Code


Summary by Gitar

  • VSIX install routing:
    • Added GetVsixInstallationTarget so VS 17.0 uses /appidinstallpath and /skuName, keeping the experimental profile
    • Other VS versions keep the /instanceIds install path

This will update automatically on new commits.

Comment thread tests/e2e/VisualStudioSession.cs Resolved
@sophio-japharidze-sonarsource
sophio-japharidze-sonarsource force-pushed the codex/slvs-3097-connected-smoke-spike branch 2 times, most recently from 32ab64c to cbc7638 Compare November 12, 2026 16:10
@sophio-japharidze-sonarsource
sophio-japharidze-sonarsource force-pushed the codex/slvs-3097-e2e-vs17-vs18-matrix branch from d69299a to 2ec9f74 Compare November 12, 2026 16:10
@sophio-japharidze-sonarsource
sophio-japharidze-sonarsource force-pushed the codex/slvs-3097-connected-smoke-spike branch from cbc7638 to 7358698 Compare November 12, 2026 16:28
@sophio-japharidze-sonarsource
sophio-japharidze-sonarsource force-pushed the codex/slvs-3097-e2e-vs17-vs18-matrix branch from 2ec9f74 to 5ef4d23 Compare November 12, 2026 16:28
Select the target IDE and its DTE registration by the major version of
E2E_EXPECTED_VS_VERSION instead of hard-coding VS 18. The VS 17.0 RTM
VSIXInstaller ignores /rootSuffix with /instanceIds and installs into the
normal profile, so target that release by application path and SKU, which
respects the experimental profile.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…8.12

Temporary matrix to validate the earliest-supported Visual Studio image from
SonarSource/ci-ami-images#520 alongside the promoted VS 18.12 master image.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sophio-japharidze-sonarsource
sophio-japharidze-sonarsource force-pushed the codex/slvs-3097-e2e-vs17-vs18-matrix branch from 5ef4d23 to 553a8b4 Compare November 12, 2026 16:50
@sonarqubecloud

Copy link
Copy Markdown

@gitar-bot

gitar-bot Bot commented Nov 12, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 🟡 Medium risk 1 closed / 1 findings

Adding VS 17.0-specific VSIX routing and matrix E2E jobs could install the extension into an unintended profile or miss IDE automation.

Adds VS 17.0 as an E2E target by routing the harness to select the IDE by its expected major version and adding a VSIX install path for VS 17.0 that uses /appidinstallpath and /skuName. The DTE ROT lookup that hardcoded VS 18 has been fixed so VS 17 jobs can attach. No open issues found.

✅ 1 closed
✅ Bug: DTE ROT lookup still hardcodes VS 18, so VS 17 jobs can never attach

📄 tests/e2e/VisualStudioSession.cs:126-133 📄 tests/e2e/VisualStudioSession.cs:177-181 📄 .github/workflows/extension-e2e.yml:23-25 📄 .github/workflows/extension-e2e.yml:93-95
The harness now picks the IDE using the major version in E2E_EXPECTED_VS_VERSION, but VisualStudioAutomation.FindDte still only accepts Running Object Table monikers that start with !VisualStudio.DTE.18.0:. VS 2022 (17.x) registers as !VisualStudio.DTE.17.0:<pid>. On the new VS 17.0 matrix legs, install and launch work, then WaitUntil("the launched IDE's DTE object", ...) at VisualStudioSession.cs:177-181 keeps getting null until it times out. Both the standalone and connected-mode E2E jobs fail before any test step runs, so this PR cannot validate the #520 image. Pass the expected major version into FindDte, as was done for the vswhere filter and profile directory, and build the prefix from it.

Verification Rules 2/2 met

Functional validation No results

Rules 2 passed 2 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant