Skip to content

Conversation

@mpoeter
Copy link
Contributor

@mpoeter mpoeter commented Dec 5, 2025

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.

  • CI pipeline:
    • Replace nightly param with full in .circleci/config.yml and .circleci/base_config.yml; adjust printing, cancellation, and generator invocation accordingly.
    • Pass --full and --arangod-without-v8 flags conditionally to the generator; drop nightly-specific handling.
  • Config generator & models:
    • Introduce TestRequirements and a requires section (job/suite) for filters (full, coverage, instrumentation, v8, arch).
    • Move filter fields out of TestOptions; make suite-level TestOptions.from_dict avoid default size so job-level size can inherit.
    • Simplify CLI: remove --single, --cluster, --all, --nightly; --arangod-without-v8 now boolean and drives v8 flag; cleanup unused fields.
  • Filtering:
    • Rework FilterCriteria (add v8, coverage, deployment_type; remove platform flags and nightly); implement requirement-based matching and inheritance to suites.
  • CircleCI output generation:
    • Use is_full_run for nightly naming; propagate architecture-specific filtering via requires.
  • Test definitions (YAML):
    • Update tests/test-definitions.yml and tests/ui.yml to use requires: (e.g., coverage: false, full: true, arch: x64, v8: true).
  • Unit tests:
    • Extensive updates to reflect new requires model, new filters, inheritance, and CLI changes.

Written by Cursor Bugbot for commit d802cde. This will update automatically on new commits. Configure here.

@mpoeter mpoeter requested a review from dothebart December 5, 2025 11:00
@cla-bot cla-bot bot added the cla-signed label Dec 5, 2025
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
Copy link

@cursor cursor bot left a 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

# Create filter criteria
filter_criteria = FilterCriteria(
single=single,
cluster=cluster,
gtest=gtest,
full=full,
all_tests=all_tests,
sanitizer=sanitizer_enum,
)

Fix in Cursor Fix in Web


- 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
Copy link

@cursor cursor bot left a 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.
Copy link

@cursor cursor bot left a 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

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)]

Fix in Cursor Fix in Web


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.
@mpoeter mpoeter merged commit 195ccd1 into devel Dec 12, 2025
9 checks passed
@mpoeter mpoeter deleted the code/COR-52-improve-test-definitions branch December 12, 2025 14:31
options:
coverage: false
priority: 1000
type: mixed
Copy link

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.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants