Skip to content

fix(sequencer-utils): fixes issue in parse_blob tests#1243

Merged
Fraser999 merged 1 commit intomainfrom
fraser/fix-parse-blob-test
Jul 9, 2024
Merged

fix(sequencer-utils): fixes issue in parse_blob tests#1243
Fraser999 merged 1 commit intomainfrom
fraser/fix-parse-blob-test

Conversation

@Fraser999
Copy link
Copy Markdown
Contributor

Summary

This PR fixes a bug in the parse_blob tests.

Background

The existing tests use colour::force_no_colour() to avoid sequencer-utils from generating coloured output in order to make the snapshots more legible. However, setting that actually has no effect (under the hood, colour was deciding to not use coloured output due to io::stdout().is_terminal() returning false).

By updating to colour v2.1.0 this becomes apparent, as that version also takes the TERM env var into account. TERM is set to a value other than dumb, so coloured output is emitted.

Changes

  • The fix is to set the NO_COLOR env var for the subprocess, as this has highest priority in disabling coloured output.

Testing

The tests broken after updating colour now pass.

@Fraser999 Fraser999 requested a review from a team as a code owner July 8, 2024 12:14
@Fraser999 Fraser999 requested a review from noot July 8, 2024 12:14
@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Jul 8, 2024
@Fraser999 Fraser999 added this pull request to the merge queue Jul 9, 2024
Merged via the queue into main with commit da40cc2 Jul 9, 2024
@Fraser999 Fraser999 deleted the fraser/fix-parse-blob-test branch July 9, 2024 15:35
steezeburger added a commit that referenced this pull request Jul 15, 2024
* main:
  feat(cli): add cmd to collect withdrawal events and submit as actions (#1261)
  fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266)
  fix(withdrawer): support withdrawer address that differs from bridge address   (#1262)
  (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260)
  fix(charts): add resources for sequencer/cometbft (#1254)
  chore(sequencer)!: add metrics (#1248)
  fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243)
  feat(core, proto)!: make bridge unlock memo string (#1244)
  fix(conductor): don't panic during panic (#1252)
  feat(core)!: lowerCamelCase for protobuf json mapping (#1250)
  refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190)
  fix: rollup archive node configurations (#1249)
  refactor(core, bridge-withdrawer)!: move bridge-unlock memo to core (#1245)
  fix(sequencer)!: store native asset ibc->trace mapping in init_chain (#1242)
steezeburger added a commit that referenced this pull request Jul 19, 2024
* main: (24 commits)
  chore: update `bytes` and `ics23` crates (#1279)
  fix(sequencer): improve and fix instrumentation (#1255)
  feature(charts): hermes chart fixes, bech32 updates, ibc bridge test (#1130)
  chore(cli): remove unused rollup cli code (#1275)
  chore(test): use a temporary file to not pollute the workspace (#1269)
  chore(sequencer): add mempool benchmarks (#1238)
  fix(bridge-withdrawer)!: fix nonce handling (#1215)
  feat(cli, bridge-withdrawer)!: share code between cli and service (#1270)
  feat(cli): add cmd to collect withdrawal events and submit as actions (#1261)
  fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266)
  fix(withdrawer): support withdrawer address that differs from bridge address   (#1262)
  (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260)
  fix(charts): add resources for sequencer/cometbft (#1254)
  chore(sequencer)!: add metrics (#1248)
  fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243)
  feat(core, proto)!: make bridge unlock memo string (#1244)
  fix(conductor): don't panic during panic (#1252)
  feat(core)!: lowerCamelCase for protobuf json mapping (#1250)
  refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190)
  fix: rollup archive node configurations (#1249)
  ...
bharath-123 pushed a commit that referenced this pull request Jul 25, 2024
## Summary
This PR fixes a bug in the `parse_blob` tests.

## Background
The existing tests use `colour::force_no_colour()` to avoid
`sequencer-utils` from generating coloured output in order to make the
snapshots more legible. However, setting that actually has no effect
(under the hood, `colour` was deciding to not use coloured output due to
`io::stdout().is_terminal()` returning `false`).

By updating to `colour` v2.1.0 this becomes apparent, as that version
also takes the `TERM` env var into account. `TERM` is set to a value
other than `dumb`, so coloured output is emitted.

## Changes
- The fix is to set the `NO_COLOR` env var for the subprocess, as this
has highest priority in disabling coloured output.

## Testing
The tests broken after updating `colour` now pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sequencer pertaining to the astria-sequencer crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants