Skip to content

fix(test): use correct conditions to dermine smoke test success#1395

Merged
SuperFluffy merged 2 commits intomainfrom
ENG-738/fix-bad-smoke-test-success-condition
Aug 23, 2024
Merged

fix(test): use correct conditions to dermine smoke test success#1395
SuperFluffy merged 2 commits intomainfrom
ENG-738/fix-bad-smoke-test-success-condition

Conversation

@SuperFluffy
Copy link
Copy Markdown
Contributor

@SuperFluffy SuperFluffy commented Aug 22, 2024

Summary

Fixes the conditionals that determine whether a smoke test passes.

Background

Many smoke tests incremented a fetch-counter as long as it was counter < max, but the test was for counter > max, which was never reached, and hence the test passed succesfully even though it actually failed. This was easily overlooked because so far the tests make use of the posix-compatible [ $counter -lt $max_counter ] and [ $counter -gt $max_counter]. But since all tests require bash this patch changes it to use (( )).

Changes

  • Change all smoke test scripts to use the the bashism (( $counter < $max_counter )) for incrementing the counter, and (( $counter >= $max_counter )) to check for failure.

Testing

This correctly fails tests if the conditions don't apply (tested offline and verified in CI).

Related Issues

Partially addresses #1392.
This patch will fail CI until after #1393 is merged.

@github-actions github-actions bot added the cd label Aug 22, 2024
@SuperFluffy SuperFluffy marked this pull request as ready for review August 22, 2024 17:28
@SuperFluffy SuperFluffy requested a review from a team as a code owner August 22, 2024 17:28
@SuperFluffy SuperFluffy enabled auto-merge August 23, 2024 16:10
@SuperFluffy SuperFluffy added this pull request to the merge queue Aug 23, 2024
Merged via the queue into main with commit 42deb6e Aug 23, 2024
@SuperFluffy SuperFluffy deleted the ENG-738/fix-bad-smoke-test-success-condition branch August 23, 2024 16:24
steezeburger added a commit that referenced this pull request Aug 26, 2024
* main:
  fix(test): use correct conditions to dermine smoke test success (#1395)
  fix(test): fail smoke test scripts eagerly if a command fails (#1394)
  fix(cli, tests): add force flag to overwrite withdrawal target path (#1393)
  release(charts): update with biweekly image cuts (#1399)
  Chore(Charts): seq-faucet bech32m chart update (#1301)
  chore: preview environment with astria-geth changes (#1401)
  release: biweekly release cut (#1398)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants