Skip to content
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

[WPT/common/security-features] Allow excluded_tests based on expectation value #21185

Merged
merged 1 commit into from Jan 15, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jan 15, 2020

Previously: excluded_tests excluded selections
that have the same values for the fields included in selection_pattern.

However, as selection_pattern doesn't contain expectation,
expectation can't be used in excluded_tests, blocking [1].

After this CL: excluded_tests excluded selections
that have the same values for all the fields (except for name).

For this purpose, this CL:

  • Sets delivery_key for excluded_selection, so that
    selections and excluded selections have the same set of fields.
  • Uses dump_test_parameters() rather than selection_pattern
    for exclusion matching (exclusion_dict).
  • Applies exclusions after all overrides are processed.
    This is a fix for cases like:
    • "default" selection A
    • "override" selection B, overriding A
    • exclusion pattern E, matching B but not A
      Previously: E excludes B but not A, so A is generated.
      After this CL: B overrides A, then E excludes B.
      As A is already overridden, no selections are generated.

This CL doesn't change generated results, because before [1]
there are no cases hitting these issues.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1991066

Bug: 906850
Change-Id: Ifa36167df710edd89d4e572346a3f1b928710119
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1991074
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731904}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

…tation` value

Previously: `excluded_tests` excluded selections
that have the same values for the fields included in `selection_pattern`.

However, as `selection_pattern` doesn't contain `expectation`,
`expectation` can't be used in `excluded_tests`, blocking [1].

After this CL: `excluded_tests` excluded selections
that have the same values for all the fields (except for `name`).

For this purpose, this CL:

- Sets `delivery_key` for `excluded_selection`, so that
  selections and excluded selections have the same set of fields.
- Uses `dump_test_parameters()` rather than `selection_pattern`
  for exclusion matching (`exclusion_dict`).
- Applies exclusions after all overrides are processed.
  This is a fix for cases like:
  - "default" selection A
  - "override" selection B, overriding A
  - exclusion pattern E, matching B but not A
  Previously: E excludes B but not A, so A is generated.
  After this CL: B overrides A, then E excludes B.
    As A is already overridden, no selections are generated.

This CL doesn't change generated results, because before [1]
there are no cases hitting these issues.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1991066

Bug: 906850
Change-Id: Ifa36167df710edd89d4e572346a3f1b928710119
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1991074
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731904}
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.

None yet

3 participants