Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix create-testnet-data creating negative supply #599

Conversation

carbolymer
Copy link
Contributor

Changelog

- description: |
    Fix `create-testnet-data` creating negative supply
# 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
  # - improvement    # QoL changes e.g. refactoring
   - 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

create-testnet-data was creating negative supply which was preventing node from start

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff


-- We test that the command doesn't crash, because otherwise
-- execCardanoCLI would fail.
hprop_create_testnet_data_create_nonegative_supply :: Property
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
hprop_create_testnet_data_create_nonegative_supply :: Property
-- Execute this test with:
-- @cabal test cardano-cli-test --test-options '-p "/create testnet data create nonegative supply/"'@
hprop_create_testnet_data_create_nonegative_supply :: Property

@smelc smelc force-pushed the mgalazyn/fix/make-create-testnet-data-produce-only-nonegative-supply branch from 1a99c72 to 164d8a1 Compare February 7, 2024 15:44
@carbolymer carbolymer force-pushed the mgalazyn/fix/make-create-testnet-data-produce-only-nonegative-supply branch 3 times, most recently from d564658 to a70fab8 Compare February 9, 2024 18:11
@carbolymer carbolymer requested a review from Jimbo4350 February 9, 2024 18:12
@carbolymer
Copy link
Contributor Author

@smelc thanks for pushing the commit with unique worksaces, but I finally reverted it. The reason is that it was circumventing the behaviour of our integration tests, which was saving only failed workspaces by default. After your change all the workspace were persisted which is too much information I think.

Additionally I am not sure about the behaviour Integration test when it's executed multiple times as a property test. This needs to be investigated more thoroughly.

@carbolymer carbolymer requested a review from Jimbo4350 February 12, 2024 08:57
@carbolymer carbolymer enabled auto-merge February 12, 2024 15:51
@carbolymer carbolymer force-pushed the mgalazyn/fix/make-create-testnet-data-produce-only-nonegative-supply branch 2 times, most recently from ada3394 to fa2a785 Compare February 12, 2024 16:15
@carbolymer carbolymer added this pull request to the merge queue Feb 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 12, 2024
@carbolymer carbolymer force-pushed the mgalazyn/fix/make-create-testnet-data-produce-only-nonegative-supply branch from fa2a785 to 1dfee0b Compare February 13, 2024 14:27
@carbolymer carbolymer enabled auto-merge February 13, 2024 14:28
@carbolymer carbolymer added this pull request to the merge queue Feb 13, 2024
Merged via the queue into main with commit 0e393cb Feb 13, 2024
16 checks passed
@carbolymer carbolymer deleted the mgalazyn/fix/make-create-testnet-data-produce-only-nonegative-supply branch February 13, 2024 14:56
@smelc smelc mentioned this pull request Feb 13, 2024
2 tasks
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.

3 participants