-
Notifications
You must be signed in to change notification settings - Fork 471
fix: use python3 -m pip install for Copilot Python SDK driver
#41303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2129,7 +2129,7 @@ func TestCopilotEngineInstallationWithCommandAndCopilotSDK(t *testing.T) { | |
| name: "python command uses pip sdk install", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] Stale test name — still says Updating test names to describe the actual behaviour keeps the test suite self-documenting and makes future regressions easier to diagnose. 💡 Suggested renamename: "python command uses python3 -m pip install", |
||
| command: "python3 main.py", | ||
| expectedName: "name: Install GitHub Copilot SDK (Python)", | ||
| expectedRun: "pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| expectedRun: "python3 -m pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| expectedSteps: 1, | ||
| }, | ||
| { | ||
|
|
@@ -2178,14 +2178,14 @@ func TestCopilotEngineInstallationWithCommandAndCopilotSDK(t *testing.T) { | |
| name: "env wrapper command is detected", | ||
| command: "env FOO=bar python script.py", | ||
| expectedName: "name: Install GitHub Copilot SDK (Python)", | ||
| expectedRun: "pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| expectedRun: "python3 -m pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| expectedSteps: 1, | ||
| }, | ||
| { | ||
| name: "custom command with firewall keeps awf and sdk installs", | ||
| command: "python script.py", | ||
| expectedName: "name: Install GitHub Copilot SDK (Python)", | ||
| expectedRun: "pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| expectedRun: "python3 -m pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| withFirewall: true, | ||
| expectedSteps: 2, | ||
| }, | ||
|
|
@@ -2249,7 +2249,7 @@ func TestCopilotEngineInstallationWithCopilotSDKDriver(t *testing.T) { | |
| name: "python driver uses pip sdk install", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale test name: 💡 Suggested renamename: "python driver uses python3 -m pip for sdk install",Consistent with fixing the sibling test name on the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] Stale test name — 💡 Suggested renamename: "python driver uses python3 -m pip install", |
||
| driver: "my_driver.py", | ||
| expectedName: "name: Install GitHub Copilot SDK (Python)", | ||
| expectedRun: "pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| expectedRun: "python3 -m pip install --disable-pip-version-check github-copilot-sdk==" + string(constants.DefaultCopilotSDKVersion), | ||
| }, | ||
| { | ||
| name: "typescript driver installs ts-node toolchain and sdk", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale test name:
"python command uses pip sdk install"no longer accurately describes the install mechanism and will mislead anyone diagnosing a future failure.💡 Suggested rename
The test now asserts
python3 -m pip install, so the name should match:Stale names make test failures harder to diagnose because the output says "pip" but the generated command says
python3 -m pip.