Skip to content

chore: fix detection of lightspeed credentials on GHA#2600

Open
ssbarnea wants to merge 1 commit intomainfrom
chore/ls-test
Open

chore: fix detection of lightspeed credentials on GHA#2600
ssbarnea wants to merge 1 commit intomainfrom
chore/ls-test

Conversation

@ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Feb 18, 2026

  • Update task ui-selenium step in CI to use ${{ env.* || secrets.* }} for LIGHTSPEED_API_ENDPOINT and LIGHTSPEED_USER.
  • Tighten sso_auth_flow parameter types/defaults for username/password.
  • Ignore .venv directories in git.
  • Add container restarting logic when test session starts
  • Avoid running coverage and xml reporting when running pytest directly (development)
  • Include only test durations for test that take more than 10s

Copy link
Contributor

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 adjusts CI and Selenium test utilities to improve how Lightspeed credentials are sourced in GitHub Actions (preferring pre-existing environment variables and falling back to secrets), and adds local venv folders to .gitignore.

Changes:

  • Update task ui-selenium step in CI to use ${{ env.* || secrets.* }} for LIGHTSPEED_API_ENDPOINT and LIGHTSPEED_USER.
  • Tighten sso_auth_flow parameter types/defaults for username/password.
  • Ignore .venv directories in git.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/selenium/utils/ui_utils.py Updates sso_auth_flow signature defaults/types for Lightspeed credentials.
.gitignore Adds .venv to ignored paths.
.github/workflows/ci.yaml Prefers existing env vars for Lightspeed endpoint/user, falls back to secrets for CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +163 to 165
username: str = LIGHTSPEED_USER,
password: str = LIGHTSPEED_PASSWORD,
*,
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

sso_auth_flow accepts username/password parameters, but the password argument is effectively ignored because it gets overwritten inside the function (password = LIGHTSPEED_PASSWORD). This makes it impossible to override credentials via arguments (and the docstring says these args are used for authentication). Consider either (a) using the passed-in args as the source of truth (falling back to env only when empty), or (b) removing the unused password parameter to avoid a misleading API.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant

Comments