chore: Update dependencies - #444
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates project dependencies and removes the legacy engine validation code from regression tests. The changes focus on maintaining only the SQL engine validation path while commenting out the old engine implementation.
Changes:
- Commented out old engine validation logic in regression tests
- Updated unit test path from
tests/unit/totests/unit-sql/in CI workflow - Added new
requirements-build.txtfile with PyInstaller dependency
Reviewed changes
Copilot reviewed 3 out of 63 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/rule_regression/regression.py | Removed old engine validation code and updated function signatures to only support SQL regression testing |
| requirements-build.txt | Added build-specific requirements file including PyInstaller 6.11.0 |
| .github/workflows/test_unit.yml | Updated test path to run SQL-specific unit tests only |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| regression_errors["dataset_import_old"] = "SUCCESS" | ||
| regression_errors["results_present_old"] = True |
There was a problem hiding this comment.
These lines set old engine status values even though the old engine code is commented out. These assignments should be removed or updated to reflect that the old engine is no longer being tested.
| regression_errors["dataset_import_old"] = "SUCCESS" | |
| regression_errors["results_present_old"] = True |
| regression_errors["old_vs_sql"] = old_vs_sql_regression_comparison(old_regression, sql_regression) | ||
| # regression_errors["old_vs_sql"] = old_vs_sql_regression_comparison(old_regression, sql_regression) | ||
|
|
||
| regression_errors["whitelisted"] = cur_core_id in WHITELISTED_RULES |
There was a problem hiding this comment.
The variable name 'cur_core_id' is not defined in the visible scope of this diff. If this is a new usage, consider using a more descriptive name. If 'cur_core_id' is defined elsewhere in the function, ensure it accurately represents the current rule identifier being processed.
| regression_errors["whitelisted"] = cur_core_id in WHITELISTED_RULES | |
| # Determine the identifier of the current rule for whitelist checking. | |
| if isinstance(rule, dict): | |
| rule_identifier = rule.get("core_id") | |
| else: | |
| rule_identifier = getattr(rule, "core_id", None) | |
| regression_errors["whitelisted"] = rule_identifier in WHITELISTED_RULES |
Updates the dependencies in the requirements file
This causes all of the old engine to break, so I've split the unit tests into original/SQL so we can continue to run the SQL unit tests