stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3k stars 659 forks source link

Update core contract tests to use the Clarinet SDK #4030

Open wileyj opened 8 months ago

wileyj commented 8 months ago

With the recent changes outlined here: https://www.hiro.so/blog/announcing-the-clarinet-sdk-a-javascript-programming-model-for-easy-smart-contract-testing

The core contract tests should be updated to use this SDK rather than relying on a soon to be outdated docker image. Currently, the only test being run is for the bns.clar contract using this step:

      - name: Execute core contract unit tests in Clarinet
        id: clarinet_unit_test
        uses: docker://hirosystems/clarinet:1.1.0
        with:
          args: test --coverage --manifest-path=./contrib/core-contract-tests/Clarinet.toml

ref: https://github.com/stacks-network/stacks-blockchain/blob/master/contrib/core-contract-tests/Clarinet.toml#L9

Future work should also be done to include other active boot contracts like pox-3.clar https://github.com/stacks-network/stacks-blockchain/tree/master/src/chainstate/stacks/boot

tagging @hugocaillard

hugocaillard commented 8 months ago

I wonder what would be the purpose of these tests. Since we are testing code that is already deployed on mainnet, it's not going to change or to break for no reason. It could be interesting to have some sort of regtests, in a sense that if the tests break for some reason , it would mean that the testing env is broken. But here the testing env is clarinet, and it does make much sense to me to run regtests on clarinet on this repo.

What do you think? cc @kantai?

I still migrated the bns test, it made sense because it was already there.

But I would be a good place to have tests for futures contracts

kantai commented 8 months ago

I think it's useful to have these tests in place even if the contract shouldn't change. Having them in place makes it easy to find tests to use as starting point for any future boot contracts, or for creating tests for bug reports in the boot contract. Running them as part of CI is useful precisely because they shouldn't fail: if they fail, they mean that a PR unexpectedly altered a boot contract. While something like that should be very easy to spot during code review, the fact that a simple CI check would fail is helpful. This CI task is also very lightweight -- it's one of the fastest CI tasks in the repo currently.

hugocaillard commented 8 months ago

Sounds good @kantai 👍

For future reference, I'm linking here @moodmosaic's suggestion to introduce tests with fast-check: https://github.com/stacks-network/stacks-core/pull/4031#discussion_r1382087703 https://github.com/stacks-network/stacks-core/pull/4031#discussion_r1382090406

moodmosaic commented 8 months ago

Sounds good @kantai 👍

For future reference, I'm linking here @moodmosaic's suggestion to introduce tests with fast-check: #4031 (comment) #4031 (comment)

That, and invariant tests as we do for sBTC, e.g. https://github.com/stacks-network/sbtc/pull/352