search

TrueBitFoundation / truebit-os

[DEPRECATED] Interactive client
Apache License 2.0
82 stars 26 forks source link

Add submodule dependency unit tests to CI #30

Closed hswick closed 5 years ago

hswick commented 6 years ago

Before truebit-os integration tests run, the unit tests for the submodule dependencies should run.

Benefits: See if submodule is failing to help with integration Central place to see health of TrueBit project

OR13 commented 6 years ago

related to this, truebit-os has a large number of submodule dependencies, some of which need to be built and tested before e2e integration tests can be run.

I recommend a structure like we have for the toolchain, with a modules directory in the root of the repo.

Then the travis integration test flow can be as follows:

  1. install global dependencies
  2. install submodule service dependencies (ipfs and ganache)
  3. test each submodule
  4. build each submodule
  5. truebit-os integration tests

3 & 4 will vary depending on the nature of the module.

Imo, basic and wasm clients should reference submodules by path not contain them.

proposed directory structure:

truebit-os
│   README.md
│   .travis.yml
└───modules
│   │   README.md
│   └───ocaml-offchain
│       │   README.md
│       │   .travis.yml
│       │   ...
│   
└───src
│   └───clients
│       └───basic-client
│       │   ...

Ideally, testing, building and deploying submodules could be parallelized, if order matters, it indicates that the boundary may not be correct.

hswick commented 6 years ago

This is great! I like adding the submodules into the modules directory. And running everything in parallel.

Sidenote, basic-client is basically deprecated at this point, and I think could just be deleted at this point since it isnt adding too much value anymore. It served its purpose.

OR13 commented 6 years ago

Nows the time to remove it then, we need to get wasm coverage up, so not needing to address the basic client could save us lots of time moving forward.

  1. expand test coverage to handle challenge / verification, incentive + dispute resolution for both cases.
  2. refactor submodules directory structure and setup build + tests to gain confidence in full system.
  3. update the cli to use the wasm client / remove basic client

I think we should stark by tackling 1 + 2 (while ignoring the basic client), I'm happy to give it a shot.

potential new issues:

hswick commented 6 years ago

1 and 3 have actually been completed already.

  1. See https://github.com/TrueBitFoundation/truebit-os/blob/master/test/os-wasm-challenge.js, and https://github.com/TrueBitFoundation/truebit-os/blob/master/test/os-wasm-alphabet-challenge.js However, I'm not entirely satisfied with the types of asserts we have going on. Right now it is, basically, if it doesnt break, then it passes. However, they are more integration tests than unit tests anyways. One thing im noticing is that the travis builds arent failing even if the tests arent passing though...

  2. I added this functionality a few weeks back, but the tests might need to be updated. Haven't looked at it in a while.

In regards to deleting basic-client, we can maybe push that later this week or next as a lot of things are changing in the repo this week.

I think 2 is biggest issue coming up as well as turning it into a monolith. Like we discusses before. Was planning on tackling this next week, and think we should get it done before adding too many new features or start doing any kind of fault tolerance or testnet deployment.

OR13 commented 6 years ago

I'll take a look at added unit tests to the WIP branch.

hswick commented 6 years ago

Now that we have consolidated, this specific issue is mostly related to ocaml-offchain.