Skip to content

fix(cli, tests): add force flag to overwrite withdrawal target path#1393

Merged
SuperFluffy merged 1 commit intomainfrom
ENG-738/add_force_to_cli
Aug 23, 2024
Merged

fix(cli, tests): add force flag to overwrite withdrawal target path#1393
SuperFluffy merged 1 commit intomainfrom
ENG-738/add_force_to_cli

Conversation

@SuperFluffy
Copy link
Copy Markdown
Contributor

@SuperFluffy SuperFluffy commented Aug 22, 2024

Summary

Adds a flag --force/-f to astria-cli bridge collect-withdrawals to overwrite the output target if it exists.

Background

When running the smoke CLI test a temporary file is created to pass to astria-cli bridge collect-withdrawals, which immediately fails because it refuses to overwrite an existing file. This patch adds a flag to overwrite it.

Changes

  • Add a flag -f/--force to the astria-cli bridge collect-withdrawals subcommand to overwrite the file passed to it

Testing

This fixes a broken test.

Related Issues

Partially addresses #1392

@github-actions github-actions bot added the cd label Aug 22, 2024
@SuperFluffy SuperFluffy added this pull request to the merge queue Aug 23, 2024
Merged via the queue into main with commit dbf6991 Aug 23, 2024
@SuperFluffy SuperFluffy deleted the ENG-738/add_force_to_cli branch August 23, 2024 16:08
github-merge-queue bot pushed a commit that referenced this pull request Aug 23, 2024
## Summary
Fail smoke tests eagerly if one command fails.

## Background
Smoke test scripts kept executing even though some of their commands
failed. This patch uses the bash built-in `set -e` to immediately exit
tests with non-zero exit code.

## Changes
- Use `set -e` in all smoke tests.

## 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-merge-queue bot pushed a commit that referenced this pull request Aug 23, 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.
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)
sgranfield4403-3 added a commit to sgranfield4403-3/astria that referenced this pull request Oct 2, 2025
## Summary
Fail smoke tests eagerly if one command fails.

## Background
Smoke test scripts kept executing even though some of their commands
failed. This patch uses the bash built-in `set -e` to immediately exit
tests with non-zero exit code.

## Changes
- Use `set -e` in all smoke tests.

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

## Related Issues
Partially addresses astriaorg/astria#1392.
This patch will fail CI until after
astriaorg/astria#1393 is merged.
sgranfield4403-3 added a commit to sgranfield4403-3/astria that referenced this pull request Oct 2, 2025
## 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 astriaorg/astria#1392.
This patch will fail CI until after
astriaorg/astria#1393 is merged.
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