0xSpaceShard / starknet-hardhat-plugin

A plugin for integrating Starknet tools into Hardhat projects
https://0xspaceshard.github.io/starknet-hardhat-plugin/
MIT License
197 stars 37 forks source link

Make running tests easier #290

Closed shramee closed 1 year ago

shramee commented 1 year ago

Running tests involves a lot of friction this makes the dev cycle slower. I have these ideas,

  1. Dockerize tests - This is a quick fix to avoid the need to having a specific environment to run the tests.
  2. Mock axios to reduce dependencies - This might require quite some work and also might need rewriting existing tests, pretty robust but would need quite some time.
  3. Document existing testing process if we want to stick to it - Minimal time.
  4. New set of mocked tests - This is least friction route which is also pretty robust.

1. Dockerize the tests

A priviledged Alpine/Debian container to run the tests.

  1. One time setup (the image and container)
    
    # Build the image
    docker build -t starknet-hardhat-plugin-tester .

Uncomment the line below to remove the old container

docker rm starknet-hardhat-plugin-tester

Create a new container

docker create --name starknet-hardhat-plugin-tester starknet-hardhat-plugin-tester


3. Running tests.
```sh
# Copy the latest plugin
docker cp . starknet-hardhat-plugin-tester:/node

# The new image will have the files
docker run -it starknet-hardhat-plugin-tester /bin/sh
  1. Then inside container we just do npm i and npm run test-general-tests etc. to run tests.

2. Mock devnet etc dependencies

We might have discussed this before and decided to go the other way, let me know what you guys think.

An Axios mock would be really neat. This would allows tests to run much faster and be great for TDD. So for example a test for devnet.mint #285 would just check if the mint method makes a request to /mint endpoint with the correct data. The drawback would be if devnet changes the way request to mint tokens is handled the plugin might be broken while the tests still pass. However with devnet instance locked to a specific version we should be all good.

3. Add documentation for running tests

Maybe I'm just missing a tiny piece somewhere, and we might just need to document the process.

4. New script to run tests with mocks

This is partial implementation of 2 for new tests, so we leave all tests as is but just write a new script that makes it super easy to write and run tests with mocked axios.

FabijanC commented 1 year ago

@shramee Thanks for all the suggestions!

  1. I think this would be a good idea, but in this comment on #285 you mentioned something about not being able to use a container?
  2. Axios mock - for now I would not go with this option due to the drawback you mentioned.
  3. Documentation
  4. Not sure what you mean by this point - you mean we keep all the tests as they are, but also introduce tests relying on the axios mock?

I am somehow closest to option 1. (of course, with documentation being updated accordingly)

shramee commented 1 year ago

Hi @FabijanC,

For 4, Yes. We add a test suite that uses a mocked Axios for much faster tests. It's separate from other tests.

Yeah, I think 1 is a pretty solid step forward to making the tests easier to run.

FabijanC commented 1 year ago

@shramee Did you plan to work on this? I can put the OnlyDust label and assign you

shramee commented 1 year ago

Aye, I do.

FabijanC commented 1 year ago

@shramee I just realized this might not be so easy (or possible) to implement. Since the a large part of the tests relies on the dockerized version of plugin, we might not be able to write a separate Docker setup that will also use another Docker container, right?

Related links: https://shard-labs.github.io/starknet-hardhat-plugin/docs/dev/#wrapper https://shramee.github.io/starknet-hardhat-plugin/docs/intro#cairo-version

FabijanC commented 1 year ago

@shramee Any update?

shramee commented 1 year ago

@FabijanC! Tru dat...

In past we did that by sharing docker socket path with the docker container, this allowed it to access host's docker easily.

Now docker has --priviledged flag that allows containers to access host's docker daemon (and more). https://docs.docker.com/engine/reference/commandline/run/#-full-container-capabilities---privileged

But still it might require quite some networking and stuff and we might use docker-compose for orchestration but I need to work on it a bit more to have more clarity on the optimal path.

My apologies that I haven't been able to spend more time on this last week. I am more active on this now and will share updates in the coming days.

FabijanC commented 1 year ago

@shramee Ok, thanks for replying, let me know if you get stuck.

shramee commented 1 year ago

Requires Shard-Labs/cairo-cli-docker#6 to run on arm based Macs.

FabijanC commented 1 year ago

Requires Shard-Labs/cairo-cli-docker#6 to run on arm based Macs.

I agree that implementing that proposal would be good, but can we work around it? In some places in the code we already do check the user's system architecture and fetch based on that: https://github.com/Shard-Labs/starknet-hardhat-plugin/blob/7efb671fd8db422eb5c0dafdfba34c87033f12d7/src/utils.ts#L241

shramee commented 1 year ago

Yeah, I think we can do that. Actually my recent tests seemed to suggest it wasn't a showstopper after all...