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

[QT-709] Validate scenario samples #134

Merged
merged 1 commit into from
May 2, 2024
Merged

[QT-709] Validate scenario samples #134

merged 1 commit into from
May 2, 2024

Conversation

ryancragun
Copy link
Collaborator

@ryancragun ryancragun commented May 1, 2024

A week or two ago we changed some scenario variants in an older Vault branch but failed to update samples to remove some variants that had been deleted.

Because the matrices on sample subsets are inclusive instead of exclusive, any subset that contained the non-existent variants ended up with an empty subset sample field. When we tried to take an observation from an empty field it would result in a singular sample of just the scenario name. In practice that meant we tried to run every variant combination instead of a single sample element in the pipeline.

This changes our behavior to disallow sample fields that are empty by default. We now specifically consider whether or not the scenario has a matrix when determining whether a sample field is a scenario with no variants or subsets have excluded all of them.

We add support for validating sample fields into both enos scenario validate and enos scenario sample observe commands.

By default enos scenario validate will now ensure that all sample and sample subset fields are valid. You can now disable either scenario or sample validation by using the --no-samples or --no-scenarios flag.

The validation command still expects a scenario filter as its default arg for filtering scenarios. If you wish to target sample validation to specific sample or sample subset you can use the new --sample-name, --include and --exclude flags.

  • validate: add support for validating samples into sample command
  • acceptance: add invalid scenario sample combination to validate suite
  • acceptance: add invalid scenario sample combination to sample observe suite
  • make: Don't run Go test in verbose mode by default
  • acceptance: split stdout and stderr in the enos acceptance runner

Checklist

  • The commit message includes an explanation of the changes
  • Manual validation of the changes have been performed (if possible)
  • New or modified code has requisite test coverage (if possible)
  • I have performed a self-review of the changes
  • I have made necessary changes and/or pull requests for documentation
  • I have written useful comments in the code

@ryancragun ryancragun added changelog/bug Fix for something that wasn't working. Will be included in "Bug Fixes" category in release notes. changelog/feat New feature or enhancement. Will be included in "New Features" category in release notes. labels May 1, 2024
@ryancragun ryancragun requested a review from a team as a code owner May 1, 2024 23:15
Copy link
Contributor

@rebwill rebwill left a comment

Choose a reason for hiding this comment

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

Neat idea! And a good call, as this (missing an update to the samples) could certainly happen again at some point.

A week or two ago we changes some scenario variants in an older Vault
branch but failed to update samples that expected deleted variants.

Because the matrices on sample subsets are _inclusive_ instead of
_exclusive_, any subset that contained the non-existent variants ended
up with an empty subset sample field. When we tried to take an
observation from an empty field it would result in a singular sample of
just the scenario name. In practice that meant we tried to run _every_
variant combination instead of a single sample in the pipeline.

This changes our behavior to disallow sample fields that are empty by
default. We now specifically consider whether or not the scenario has a
matrix when determining whether a sample field is a scenario with no
variants or subsets have excluded all of them.

We add support for validating sample fields into both `enos scenario
validate` and `enos scenario sample observe` commands.

By default `enos scenario validate` will now ensure that all sample and
sample subset fields are valid. You can now disable either scenario or
sample validation by using the `--no-samples` or `--no-scenarios` flag.

The validation command still expects a scenario filter as its default
arg for filtering scenarios. If you wish to target sample validation to
specific sample or sample subset you can use the new `--sample-name`,
`--include` and `--exclude` flags.

* validate: add support for validating samples into sample command
* acceptance: add invalid scenario sample combination to validate suite
* acceptance: add invalid scenario sample combination to sample observe
  suite
* make: Don't run Go test in verbose mode by default
* acceptance: split stdout and stderr in the enos acceptance runner

Signed-off-by: Ryan Cragun <[email protected]>
@ryancragun ryancragun merged commit f9717ae into main May 2, 2024
5 checks passed
@ryancragun ryancragun deleted the ryan/qt-709 branch May 2, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/bug Fix for something that wasn't working. Will be included in "Bug Fixes" category in release notes. changelog/feat New feature or enhancement. Will be included in "New Features" category in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants