-
Notifications
You must be signed in to change notification settings - Fork 706
date64 test skip for 24-4 #21609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
date64 test skip for 24-4 #21609
Conversation
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
🟢 |
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
]) | ||
def test_tpch1(self, store_type, date_args): | ||
def test_tpch1(self, store_type, date_args, is_date64): | ||
if is_date64 and min(self.versions) < (25, 1): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну как-то избыточно получается. Давай date_args тогда удалим из параметров теста и будем крафтить эту переменную в тесте на основе is_date64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Поправил
There was a problem hiding this 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 implements conditional skipping for date64 test cases in compatibility tests for YDB version 24-4. The changes ensure that tests using 64-bit datetime types are skipped when running against older YDB versions that don't support this feature.
- Added version-based skipping logic for date64 tests
- Updated test parameterization to include a date64 flag for better control
- Removed outdated comments referencing 24-4 limitations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
ydb/tests/compatibility/test_stress.py | Added is_date64 parameter and skip logic for test_tpch1 and test_tpcds1 methods |
ydb/tests/compatibility/test_compatibility.py | Added is_date64 parameter and skip logic for test_tpch1 method |
@pytest.mark.parametrize("store_type, date_args, is_date64", [ | ||
pytest.param("row", ["--datetime-types=dt32"], False, id="row"), | ||
pytest.param("column", ["--datetime-types=dt32"], False, id="column"), | ||
pytest.param("column", ["--datetime-types=dt64"], True, id="column-date64") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Inconsistent spacing in the True parameter. Should use single space like other parameters for consistency.
pytest.param("column", ["--datetime-types=dt64"], True, id="column-date64") | |
pytest.param("column", ["--datetime-types=dt64"], True, id="column-date64") |
Copilot uses AI. Check for mistakes.
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Changelog entry
...
Changelog category
Description for reviewers
...