Skip to content

Conversation

@d0rich
Copy link
Contributor

@d0rich d0rich commented Jul 14, 2025

No description provided.

@d0rich d0rich self-assigned this Jul 14, 2025
@d0rich d0rich requested review from anngoroshi and Copilot July 14, 2025 05:14
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 adds comprehensive tests for SQLite-backed tables and utilities, enforces a minimum SQLite version at runtime, reorganizes dependencies into groups, and updates the CI workflow.

  • Added new unit tests for Semver, embeddings, and all database tables under tests/test_db/
  • Introduced a SQLite version check and foreign-key enforcement in DbClient
  • Split dependencies into production and dev groups in pyproject.toml and updated GitHub Actions

Reviewed Changes

Copilot reviewed 19 out of 23 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_semver.py Added unit tests for Semver parsing and comparison
tests/test_embeddings.py Updated logging level and skip logic for embedding tests
tests/test_db_client.py Removed legacy top-level DB client test
tests/test_db/test_db_client.py Added DB client extension and version check test
tests/test_db/test_tables/test_test_cases.py New tests for TestCases table insert/get_or_insert behavior
tests/test_db/test_tables/test_requirements.py New tests for Requirements table insert scenarios
tests/test_db/test_tables/test_cases_to_annos.py New tests for CasesToAnnos table insert and count
tests/test_db/test_tables/test_annotations.py New tests for Annotations table insert/get_or_insert and embeddings
tests/test_db/test_tables/test_annos_to_reqs.py New tests for AnnosToReqs table insert and count
test2text/utils/semver.py Implemented Semver class with comparison operators
test2text/db/tables/test_case.py Added return typing and get_or_insert logic for TestCases table
test2text/db/tables/cases_to_annos.py Enhanced insert/count and added recreate_table for CasesToAnnos
test2text/db/tables/annotations.py Added return typing and get_or_insert logic for Annotations table
test2text/db/tables/annos_to_reqs.py Enhanced insert/count for AnnotationsToRequirements table
test2text/db/client.py Added SQLite version check and foreign-key pragma enabling
pyproject.toml Separated dependencies into production/dev groups
index_requirements.py Switched to get_or_insert for requirements indexing
index_annotations.py Switched to get_or_insert for annotations and test cases
.github/workflows/test.yaml Configured CI workflow to install dependencies and run tests
Comments suppressed due to low confidence (4)

.github/workflows/test.yaml:21

  • The CI installs only the production group and omits dev dependencies like coverage, which will cause the test step to fail; consider syncing all or including the dev group.
          run: uv sync --no-group production

test2text/db/tables/test_case.py:36

  • The else: in get_or_insert is mis-indented relative to its if and will cause a syntax error; align it with the if inserted_id is not None: block.
        else:

test2text/db/tables/annotations.py:31

  • Optional is used in the return type but not imported; add from typing import Optional to avoid a NameError.
    def insert(self, summary: str, embedding: list[float] = None) -> Optional[int]:

index_requirements.py:23

  • The get_or_insert call likely doesn’t accept positional embedding and external_id arguments in this order; verify its signature and use named parameters or correct ordering.
                db.requirements.get_or_insert(requirement, embedding, external_id)

@d0rich d0rich requested a review from Copilot July 14, 2025 15:54
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@d0rich d0rich merged commit f9346c1 into master Jul 14, 2025
1 check passed
anngoroshi pushed a commit that referenced this pull request Jul 18, 2025
* Add possibility to run embedding test without model

* Move DB client tests to submodule

* Refactor annotations table

* Refactor test cases table

* Tests for annotations table

* Fix embeddings test

* TestCases table test

* Requirements table tests

* Turn on foreign keys and CasesToAnnos table tests

* CasesToAnnos table - add insertion fact return

* AnnosToReqs table tests

* Semver implementation

* Check SQLite version

* Move torch dependencies to production group

* Switch logging level to warning

* Configure tests CI

* Fix coverage script

* Ignore errors for coverage

* Run tests without coverage

* Revert "Run tests without coverage"

This reverts commit 1b895f6.

* Try avoid torch installation in uv

* Check only project files

* Do not download production dependencies on coverage report generation

* Fix wrong method call

* Install ruff

* Lint in CI

* Configure ruff linter

* Check formatting in CI

* Format files

* Add last empty lines to files

* Correct old SQLite error message
@d0rich d0rich deleted the sqlite_tests branch August 5, 2025 14:23
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