rmrk-team / rmrk-substrate

[WIP] RMRK Substrate pallets
https://rmrk.app
Other
74 stars 36 forks source link

fix issue 208 allow minting after burn #215

Closed bmacer closed 2 years ago

bmacer commented 2 years ago

addresses #208

bmacer commented 2 years ago

I merged main into this branch, then went ahead trying to fix the integration test. But it didn't seem to fail for me. I'd be surprised if the merge from main fixed it.

Can you double-check the integration test is failing on your end?

bmacer@BMACER-M-XRG9 tests % git branch
* bug/208-locked-collection-allow-minting-after-burn
  main
bmacer@BMACER-M-XRG9 tests % yarn testLockCollection
yarn run v1.22.18
$ mocha --timeout 9999999 -r ts-node/register './src/lockCollection.test.ts'

  integration test: lock collection
    ✔ lock collection (2168ms)
    ✔ [negative] lock non-existing NFT collection (1996ms)
    ✔ [negative] lock not an owner NFT collection issuer (3991ms)
    ✔ lock collection with minting (14017ms)
    ✔ [negative] unable to mint NFT inside a locked collection (6001ms)
    ✔ [negative] unable to mint NFT inside a full collection (5996ms)

  6 passing (34s)

✨  Done in 38.46s.
ilionic commented 2 years ago

@bmacer yes integration tests pass now, but unit test fails:

failures:

---- tests::lock_collection_works stdout ----
thread 'tests::lock_collection_works' panicked at 'assertion failed: `(left == right)`
  left: `Ok(())`,
 right: `Err(Module(ModuleError { index: 2, error: [14, 0, 0, 0], message: Some("CollectionFullOrLocked") }))`', pallets/rmrk-core/src/tests.rs:149:9
bmacer commented 2 years ago

@ilionic Turns out when I merged main, I undid my changes, so we returned to the original state. I fixed this. The integration test was failing because it was checking that max for the collection == collection size, which is the logic we changed. Now in all cases, max is set to 0 on locking. I changed the integration test to reflect this, and the test testLockCollection is now passing. Also, unit tests are passing. Should be good now.