fix: Resolve merge conflicts with main branch for Rust CI integration#137
Conversation
Co-authored-by: fbkaragoz <59958216+fbkaragoz@users.noreply.github.com>
Co-authored-by: fbkaragoz <59958216+fbkaragoz@users.noreply.github.com>
Co-authored-by: fbkaragoz <59958216+fbkaragoz@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR resolves merge conflicts between Rust CI additions and the main branch by updating GitHub Actions versions, replacing a lemmatizer quality evaluation step with property-based tests, and optimizing Rust CI execution to run only once per workflow instead of for each Python version.
Changes:
- Updates GitHub Actions checkout version and adds Python 3.11 conditionals to Rust CI steps
- Replaces lemmatizer evaluation workflow step with property-based testing using Hypothesis
- Adds new test files implementing property-based tests for Turkish NLP functions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/tests.yml | Updates checkout action version, adds conditionals to Rust checks for single execution on Python 3.11, replaces lemmatizer evaluation with property-based test step |
| tests/test_properties.py | New property-based test suite using Hypothesis to verify idempotency, type safety, and edge case handling across Turkish NLP functions |
| tests/strategies.py | New Hypothesis strategy definitions for generating Turkish text test cases with proper diacritics, suffixes, and Unicode edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Run property-based tests with statistics | ||
| run: pytest tests/test_properties.py --hypothesis-show-statistics -v |
There was a problem hiding this comment.
The property-based tests require the 'hypothesis' package, but it is not included in the project's dev dependencies in pyproject.toml. This will cause the test step to fail with an import error. Add 'hypothesis>=6.0.0' to the dev dependencies list in pyproject.toml.
| # Pick a word from tokens as keep word (or use a known stopword) | ||
| keep_word = tokens[0] if tokens else "ve" |
There was a problem hiding this comment.
The ternary expression 'tokens[0] if tokens else "ve"' is redundant. Since line 164 contains 'assume(len(tokens) > 0)', tokens is guaranteed to be non-empty at this point, so the else clause will never be executed. Simplify to just 'keep_word = tokens[0]'.
| # Pick a word from tokens as keep word (or use a known stopword) | |
| keep_word = tokens[0] if tokens else "ve" | |
| # Pick a word from tokens as keep word | |
| keep_word = tokens[0] |
| @@ -47,11 +51,9 @@ jobs: | |||
| - name: Run tests with coverage | |||
| run: pytest --cov=durak --cov-report=xml --cov-report=term | |||
There was a problem hiding this comment.
The property-based tests in test_properties.py will be executed twice: once during the coverage test run on line 52, and again on line 55 with statistics. Consider adding a conditional filter to exclude test_properties.py from the coverage run (e.g., '--ignore=tests/test_properties.py') or remove the separate execution if statistics are not critical.
| run: pytest --cov=durak --cov-report=xml --cov-report=term | |
| run: pytest --cov=durak --cov-report=xml --cov-report=term --ignore=tests/test_properties.py |
The branch had unrelated histories causing merge conflicts between the Rust CI additions and changes in main.
Changes
actions/checkout@v4→@v6to align with maintest_properties.pyandstrategies.pyrequired by property-based test stepif: matrix.python-version == '3.11'to Rust checks to avoid redundant execution across Python 3.9-3.12 matrixResult
The Rust CI checks (test, clippy, fmt) now integrate cleanly with main while executing efficiently.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.