Skip to content

Improve testing support#7

Merged
AmaranthineCodices merged 4 commits intomainfrom
lbrown/testing
Jul 16, 2025
Merged

Improve testing support#7
AmaranthineCodices merged 4 commits intomainfrom
lbrown/testing

Conversation

@AmaranthineCodices
Copy link
Collaborator

This will allow us to test SDK-related features again in the aftermath of #5.

@rparolin rparolin requested a review from Copilot July 16, 2025 21:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves the testing support for SDK-related features by adding Pixi workspace testing capabilities and restructuring the test configuration to support multiple test environments.

  • Adds Pixi workspace testing infrastructure with fixture files and dedicated test cases
  • Restructures test configuration to support both default and Pixi-specific test environments
  • Updates CI workflow to include Pixi setup and workspace initialization

Reviewed Changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
package.json Simplifies test script by removing build cleanup step
fixtures/pixi-workspace/* Adds new Pixi workspace fixture with configuration, source file, and Git settings
extension/test/reporter.ts Updates test reporter formatting for pending tests
extension/*.test.pixi.ts Adds Pixi-specific test files for pyenv and LSP functionality
.vscode-test.mjs Restructures test configuration to support multiple test environments
.github/workflows/test.yaml Adds Pixi setup and workspace initialization to CI workflow


runner.on(EVENT_TEST_PENDING, (test: Test) => {
console.log(indent(`${Base.color('pending', test.title)}`));
console.log(indent(Base.color('pending', `- ${test.title}`)));
Copy link

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The change adds a hardcoded dash prefix to the test title. Consider using a consistent formatting pattern or making this configurable to maintain consistency with other test output formatting.

Suggested change
console.log(indent(Base.color('pending', `- ${test.title}`)));
console.log(indent(Base.color('pending', formatTestTitle('pending', test.title))));

Copilot uses AI. Check for mistakes.
[tasks]

[dependencies]
modular = "25.5.0.dev2025071605"
Copy link

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependency version is hardcoded to a specific development build. Consider using a more stable version or documenting the reason for this specific dev version to avoid future maintenance issues.

Suggested change
modular = "25.5.0.dev2025071605"
modular = "25.5.0"

Copilot uses AI. Check for mistakes.
const sdk = await extension.pyenvManager!.getActiveSDK();
assert.ok(sdk);
assert.strictEqual(sdk.kind, SDKKind.Environment);
assert.strictEqual(sdk.version, '25.5.0.dev2025071605');
Copy link

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test hardcodes the expected version string. This creates a tight coupling between the test and the fixture configuration, making maintenance difficult when updating versions.

Suggested change
assert.strictEqual(sdk.version, '25.5.0.dev2025071605');
const expectedVersion = await extension.pyenvManager!.getExpectedSDKVersion();
assert.strictEqual(sdk.version, expectedVersion);

Copilot uses AI. Check for mistakes.
@rparolin rparolin self-requested a review July 16, 2025 21:55
@AmaranthineCodices AmaranthineCodices merged commit 2a2eda6 into main Jul 16, 2025
2 checks passed
@AmaranthineCodices AmaranthineCodices deleted the lbrown/testing branch July 16, 2025 22:29
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.

3 participants