Skip to content

Add PR-triggered fuzzer workflow with strict validation#68

Open
sarthakaggarwal97 wants to merge 19 commits intovalkey-io:mainfrom
sarthakaggarwal97:pr-triggered-fuzzer-strict-validation
Open

Add PR-triggered fuzzer workflow with strict validation#68
sarthakaggarwal97 wants to merge 19 commits intovalkey-io:mainfrom
sarthakaggarwal97:pr-triggered-fuzzer-strict-validation

Conversation

@sarthakaggarwal97
Copy link
Collaborator

Summary

  • add a reusable/manual GitHub Actions workflow to build a Valkey PR merge ref once and fan out 10 random fuzzer runs
  • add explicit --valkey-binary support so workflow jobs run against the exact built Valkey binary
  • make run success strict: fail if any operation fails, any chaos injection fails, any post-operation validation wave fails, or final validation fails
  • validate after each parallel operation wave and surface post-operation/final validation details in workflow artifacts and summary
  • document the Valkey-side label-trigger workflow and add focused regression coverage

Testing

  • pytest tests/test_fuzzer_engine_strictness.py tests/test_state_validator.py tests/test_cli.py tests/test_orchestrator.py -q -k "not full_cluster_creation and not large"
  • python -m compileall src tests/test_fuzzer_engine_strictness.py
  • workflow YAML parse check for .github/workflows/pr-merge-fuzzer.yml

Notes

  • the new workflow remains in the fuzzer repo, but the label-trigger entrypoint still needs to live in the valkey repo because that is where the PR event originates
  • unrelated untracked local files were intentionally left out of this branch

@sarthakaggarwal97 sarthakaggarwal97 force-pushed the pr-triggered-fuzzer-strict-validation branch 2 times, most recently from 492f106 to 2075099 Compare March 11, 2026 22:25
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20750992e3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Nikhil-Manglore
Copy link
Member

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 055f8eac02

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Nikhil-Manglore
Copy link
Member

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55d0a09c36

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@sarthakaggarwal97 sarthakaggarwal97 force-pushed the pr-triggered-fuzzer-strict-validation branch from 55d0a09 to 672170a Compare March 12, 2026 20:18
@Nikhil-Manglore
Copy link
Member

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 672170ae47

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@sarthakaggarwal97
Copy link
Collaborator Author

sarthakaggarwal97 commented Mar 12, 2026

PR Summary

Introduces a reusable GitHub Actions workflow that builds a Valkey PR merge ref and fans out multiple randomized fuzzer runs against the built binary. The CLI and orchestrator are extended with --valkey-binary support so each workflow job targets the exact compiled binary. Validation is made strict—failing on any operation, chaos injection, or validation error. Supporting changes include Dependabot configuration, .gitignore fixes for .github/ paths, and updated documentation.

File Groups

Files Summary
.github/workflows/valkey-pr-fuzzer.yml New reusable workflow: accepts valkey_ref, pr_number, fuzzer_repository, etc. as inputs; builds Valkey once, then fans out N random fuzzer runs via matrix strategy; uploads per-run artifacts; strict failure semantics.
.github/dependabot.yml New Dependabot config for weekly GitHub Actions dependency updates (Monday 09:00 PT).
.gitignore Adds exclusion rules so .github/, .github/workflows/*.yml, and .github/dependabot.yml are not ignored by the existing .*/ pattern.
src/cli.py Adds --valkey-binary CLI argument to the cluster subcommand; forwards it as a keyword argument to run_random_test() and run_dsl_test().
src/cluster_orchestrator/orchestrator.py setup_valkey_from_source() now checks self.clusterConfig.valkey_binary first, validates the path exists and is executable, and only falls back to which valkey-server when no binary is configured.
README.md Documents --valkey-binary usage, PR-triggered fuzzer workflow overview, and provides an example label-trigger caller workflow for the Valkey repo.

Release Notes

New Feature

  • Added reusable GitHub Actions workflow (valkey-pr-fuzzer.yml) to build a Valkey PR and run multiple randomized fuzzer sessions against it, with per-run artifact uploads and strict pass/fail semantics.
  • Added --valkey-binary CLI option to target a specific Valkey server binary for both random and DSL-based fuzzer runs.
  • Orchestrator now validates explicitly configured binary paths for existence and executability before use.

Chore

  • Added Dependabot configuration for weekly GitHub Actions dependency updates.
  • Updated .gitignore to track .github/ workflow files.

Documentation

  • README updated with --valkey-binary examples and PR-triggered workflow setup instructions.

@sarthakaggarwal97 sarthakaggarwal97 force-pushed the pr-triggered-fuzzer-strict-validation branch from 67fae97 to ab28922 Compare March 12, 2026 21:27
if failed_chaos_events:
failure_reasons.append(f"{len(failed_chaos_events)} chaos injection(s) failed")

if validation_results:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bug: The guard if total_operations > 0 and not validation_results that reported "post-operation validation did not run" was removed, but the else keyword that gated the failed_validation_waves logic was also removed. Now failed_validation_waves is computed even when validation_results is an empty list (the falsy case), which silently skips reporting the missing-validation failure. If validation_results is empty and operations were executed, no failure reason is appended — a regression from the previous behavior that explicitly flagged this scenario.

Copy link
Member

@Nikhil-Manglore Nikhil-Manglore left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, just a minor comment. I didn't review the tests but everything else looks good.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
sarthakaggarwal97 and others added 8 commits March 13, 2026 15:05
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
When clusterConfig.valkey_binary is set but shutil.which cannot resolve
it, raise FileNotFoundError instead of silently falling back to
valkey-server from PATH or a source build. This prevents running fuzzer
tests against the wrong Valkey binary/revision.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
The old default '/usr/local/bin/valkey-server' was always truthy, so
the fail-fast check raised FileNotFoundError on CI where valkey is not
installed. Default to empty string so the fallback to PATH / source
build still works when no binary is explicitly configured.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
- orchestrator: use os.path.isfile + os.access for absolute paths instead
  of shutil.which, giving clear PermissionError when binary exists but is
  not executable
- fuzzer_engine: remove false-positive 'validation did not run' failure
  when validation_results is empty due to legitimate early termination
- update tests to match new binary resolution logic

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthakaggarwal97@gmail.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Only skip the 'post-operation validation did not run' warning when
operations failed (early termination). When all operations succeeded
but validation_results is empty, still flag it as a failure.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
…_from_source

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthakaggarwal97@gmail.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97 sarthakaggarwal97 force-pushed the pr-triggered-fuzzer-strict-validation branch from 81e353a to 7021368 Compare March 13, 2026 22:05
…_reasons

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants