Skip to content

Conversation

smelc
Copy link
Contributor

@smelc smelc commented Feb 4, 2025

Changelog

- description: |
    cardano-cli-golden: protect shared file by a semaphore, to avoid spurious failures
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

test/cardano-cli-golden/files/golden/shelley/stake-address/reg-certificate-2.json is being used by two tests and I think this has been creating flakiness in the CI, as visible here (failure happened on #1036, see here and also here on #1038):

image

How to trust this PR

  • Tests pass
  • Locking looks good

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • Self-reviewed the diff

…ley/stake-address/reg-certificate-2.json by a semaphore
@smelc smelc force-pushed the smelc/add-missing-semaphore-in-tests branch from f6bd784 to 397757f Compare February 4, 2025 15:28
@CarlosLopezDeLara CarlosLopezDeLara mentioned this pull request Feb 4, 2025
3 tasks
Copy link
Contributor

@carbolymer carbolymer left a comment

Choose a reason for hiding this comment

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

The semaphore won't help here because there's no race condition on writes here: there's only one producer of this file and one consumer. The problem is the order of operations in CI, sometimes it's being read before it's produced. We're deleting golden files prior to executing cardano-cli-golden to detect potential changes.

@smelc smelc closed this Feb 5, 2025
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