Skip to content

Conversation

@darrenge
Copy link
Collaborator

@darrenge darrenge commented Mar 20, 2025

Change to the BDN to allow to set expected values to only report as a warning instead of a fail. This is for tests that are not very deterministic / stable and we don't want false fails to be showing.

  • The results of the pipeline won't show warnings, so only way to see if warning is by opening up the results in the output / logs
  • It is better for a test to only be Warning instead of false fails.
  • These individual tests can easily be moved out of the "Warn only" if it is determined that they are stable
  • Docs on website updated too
  • The test is still ran and the data is still saved for the charts.

HashObjectOperations and SortedSetOperations have seen more sporadic results so for now, just put the whole BDN in WARN ONLY status. In future PRs, we can work to make these more deterministic. As for others that are marked for WARN ONLY, this is a result of looking at the past 20+ runs and if that specific one failed at all, I put it as WARN ONLY.

… warning instead of a fail. This is for tests that are not very deterministic / stable and we don't want false fails to be showing. The test is still ran and the data is still saved for the charts. Docs on website were updated as well.
Copilot AI review requested due to automatic review settings March 20, 2025 21:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aims to stabilize BDN tests by introducing a flag that marks certain test expectations as warnings rather than errors, preventing false pipeline failures.

  • Updated documentation to explain the new prefix (WARN-ON-FAIL_) for non-deterministic expected values.
  • Clarified behavior for tests flagged as warnings while still including their results in performance charts.
Files not reviewed (3)
  • .github/workflows/ci-bdnbenchmark.yml.bak: Language not supported
  • test/BDNPerfTests/BDN_Benchmark_Config.json: Language not supported
  • test/BDNPerfTests/run_bdnperftest.ps1: Language not supported
Comments suppressed due to low confidence (1)

website/docs/benchmarking/overview.md:33

  • [nitpick] Consider formatting the prefixes 'WARN-ON-FAIL_' and 'expected_' with code markers (e.g., WARN-ON-FAIL_) for improved clarity in documentation.
d) If the expected values are not highly deterministic, it is advisable to avoid failing the test when these values are out of compliance. To prevent false failures, prepend WARN-ON-FAIL_ instead of expected_ to each expected value. This ensures the values are checked and will only generate warnings without causing a pipeline failure. The values will still be included in the charts, even if they are marked as warnings.

@darrenge darrenge requested a review from badrishc March 20, 2025 21:01
@darrenge darrenge merged commit fb52a2b into main Mar 21, 2025
26 checks passed
@darrenge darrenge deleted the darrenge/cleanupBDN branch March 21, 2025 03:08
@github-actions github-actions bot locked and limited conversation to collaborators May 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants