-
Notifications
You must be signed in to change notification settings - Fork 875
[COR-52] Refine test-definitions format and add instrumentation filter #22161
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
Conversation
Remove --nightly parameter in favor of --full. Remove unused PlatformFlags.
Add v8 field to TestRequirements for filtering tests based on whether V8 (JavaScript) is enabled. - Add v8 field parsing and merging to TestRequirements - Add is_v8_build property to FilterCriteria - Implement v8 compatibility checking in filtering logic - Add comprehensive test coverage
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.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 28
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bug: V8 filter not connected to CLI parameter
The new V8 filtering feature is incomplete. While FilterCriteria has a v8 field and _check_requirements_match checks requires.v8 against criteria.is_v8_build, the v8 field is never set when creating FilterCriteria in create_generator_config. The arangod_without_v8 parameter is available (line 145) and used for TestExecutionConfig, but not passed to FilterCriteria. Since v8 defaults to True, tests with requires: {v8: false} would always be incorrectly filtered out even when the build has V8 disabled.
.circleci/generate_config.py#L169-L177
arangodb/.circleci/generate_config.py
Lines 169 to 177 in 9cccec8
| # Create filter criteria | |
| filter_criteria = FilterCriteria( | |
| single=single, | |
| cluster=cluster, | |
| gtest=gtest, | |
| full=full, | |
| all_tests=all_tests, | |
| sanitizer=sanitizer_enum, | |
| ) |
- Add v8 field to TestRequirements for V8/non-V8 build filtering - Connect v8 filter to CLI via FilterCriteria - Convert arangod_without_v8 from string to proper boolean flag - Add comprehensive v8 filtering tests (9 tests) - Simplify boolean logic throughout
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.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 28
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
This is not needed for circleci, but for legacy jenkins builds.
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.
Bug: Suite filtering doesn't inherit job-level requirements
The filter_suites function uses job.suites directly without merging job-level requirements into suite requirements. When a job has requires: {full: true} but individual suites have no requires section, the suites will incorrectly pass filtering in PR builds because their requires.full is None (not inherited from job). The function should use job.get_resolved_suites() instead of job.suites to ensure job-level requires fields properly propagate to suite filtering.
.circleci/src/filters.py#L183-L195
arangodb/.circleci/src/filters.py
Lines 183 to 195 in ab86115
| def filter_suites(job: TestJob, criteria: FilterCriteria) -> List[SuiteConfig]: | |
| """ | |
| Filter suites from a job based on criteria. | |
| Args: | |
| job: TestJob containing suites to filter | |
| criteria: FilterCriteria to apply | |
| Returns: | |
| Filtered list of SuiteConfig objects | |
| """ | |
| return [suite for suite in job.suites if should_include_suite(suite, criteria)] |
Use get_resolved_suites() instead of accessing job.suites directly to ensure job-level requirements (full, instrumentation, v8, etc.) are properly inherited by suites without their own requires section. Add regression test for job-level requirement inheritance.
Add coverage field to FilterCriteria and implement filtering logic to exclude tests marked with coverage:false from coverage builds. Changes: - Add coverage field to FilterCriteria (default: false) - Add is_coverage_build property to FilterCriteria - Update is_instrumented_build to include coverage builds - Add coverage checking in _check_requirements_match() - Add 9 comprehensive unit tests for coverage filtering This matches the behavior of the old Oskar script which filtered tests based on the COVERAGE environment variable.
| options: | ||
| coverage: false | ||
| priority: 1000 | ||
| type: mixed |
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.
Bug: Missing requires block for arangobench_deadlock_audit job
The arangobench_deadlock_audit job previously had coverage: false in its options section (visible in the diff as a removed line), but this was not migrated to a requires block like other jobs in this PR. This means the job will now incorrectly run in coverage builds when it was previously excluded from them.
Scope & Purpose
Introduce requires subobject and move filter conditions there
Add instrumentation filter condition
Small cleanup (remove --nightly parameter in favor of --full; remove unused PlatformFlags), remove unused parameters (--single, --cluster, --all)
🔨 Refactoring/simplification
Note
Move test filtering to a new requires section (with instrumentation/coverage/v8/arch), replace nightly with full across CI/generator, and update filtering logic, YAML definitions, and tests accordingly.
nightlyparam withfullin.circleci/config.ymland.circleci/base_config.yml; adjust printing, cancellation, and generator invocation accordingly.--fulland--arangod-without-v8flags conditionally to the generator; drop nightly-specific handling.TestRequirementsand arequiressection (job/suite) for filters (full,coverage,instrumentation,v8,arch).TestOptions; make suite-levelTestOptions.from_dictavoid default size so job-level size can inherit.--single,--cluster,--all,--nightly;--arangod-without-v8now boolean and drivesv8flag; cleanup unused fields.FilterCriteria(addv8,coverage,deployment_type; remove platform flags and nightly); implement requirement-based matching and inheritance to suites.is_full_runfor nightly naming; propagate architecture-specific filtering viarequires.tests/test-definitions.ymlandtests/ui.ymlto userequires:(e.g.,coverage: false,full: true,arch: x64,v8: true).requiresmodel, new filters, inheritance, and CLI changes.Written by Cursor Bugbot for commit d802cde. This will update automatically on new commits. Configure here.