Skip to content

Conversation

@Perdiga
Copy link
Collaborator

@Perdiga Perdiga commented Jul 8, 2025

No description provided.

Copilot AI review requested due to automatic review settings July 8, 2025 02:01
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 refactors error handling in the CodeQL installer to raise exceptions instead of exiting the process and updates the corresponding tests to expect those exceptions and pass an explicit version when invoking install().

  • Switch get_latest_version() from sys.exit to raising a generic Exception
  • Remove the now-unused import sys
  • Update and rename tests to expect exceptions and mock out get_latest_version

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/test_codeql_installer.py Tests renamed from “exits” to “raises”, updated to expect exceptions, added mocks for version
src/codeql_wrapper/infrastructure/codeql_installer.py Replaced sys.exit(1) calls with raise Exception(...), removed sys import
Comments suppressed due to low confidence (2)

tests/test_codeql_installer.py:555

  • [nitpick] The test name 'test_get_latest_version_raises_on_error' is ambiguous with other error scenarios. Consider renaming it to 'test_get_latest_version_raises_on_network_error' to clearly distinguish the network error case.
    def test_get_latest_version_raises_on_error(self) -> None:

tests/test_codeql_installer.py:661

  • There are tests covering error cases for get_download_url, but no test for the successful path. Consider adding a test that verifies a correct download URL is returned for a valid version.
    def test_get_download_url_raises_on_api_error(self) -> None:


def test_get_latest_version_exits_on_http_error(self) -> None:
"""Test get_latest_version exits program on HTTP error status."""
def test_get_latest_version_raises_on_http_error(self) -> None:
Copy link

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

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

[nitpick] These three exception tests for HTTP error, invalid JSON, and network error share almost identical setup. You could parametrize them with pytest.mark.parametrize to reduce duplication and make maintenance easier.

Copilot uses AI. Check for mistakes.
Note:
If unable to fetch the latest version, logs the error and exits the program.
Raises:
Exception: If unable to fetch the latest version from GitHub API
Copy link

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

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

Raising a generic Exception can make error handling less precise. Consider defining and raising a custom exception (e.g., CodeQLInstallerError) so callers can catch installer-specific errors explicitly.

Suggested change
Exception: If unable to fetch the latest version from GitHub API
CodeQLInstallerError: If unable to fetch the latest version from GitHub API

Copilot uses AI. Check for mistakes.
@Perdiga Perdiga merged commit db8eb51 into main Jul 8, 2025
7 checks passed
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.

2 participants