Add test coverage measurement

tkornuta-semiotic commented 1 year ago

Leverage coveralls

aasseman commented 1 year ago

I think free Coveralls would not work here since the project is not public/oss yet. But perhaps we can prepare for when that happens? Also, here's how I leveraged Coveralls before, without installing the Coveralls app in the whole org (lots of permissions): https://github.com/semiotic-ai/autoagora/blob/db30582816218b072ce37f3db16f0a85b9770825/.github/workflows/tests.yml#LL47-L51C40

pablogmorales commented 1 year ago

@aasseman I found this one that supports rust and is open source and looks pretty cool and can be installed as a cargo plugin.

https://github.com/xd009642/tarpaulin

ColePBryan commented 1 year ago

Here is another that I have been using locally. It supports llvm natively and is supposed to be more accurate than tarpaulin due to that.

https://github.com/taiki-e/cargo-llvm-cov

aasseman commented 1 year ago

Both look OK, I'll let you guys decide / compare.

ColePBryan commented 1 year ago

I don't have a preference. Whichever is easier to set up.

pablogmorales commented 1 year ago

@ColePBryan
@aasseman

I integrated llvm-cov , this is an example output of cargo llvm-cov

Compiling anes v0.1.6
   Compiling criterion v0.4.0
   Compiling rstest v0.16.0
   Compiling ethers v2.0.3
   Compiling ethers-derive-eip712 v2.0.2
error[E0425]: cannot find function `parse_fields` in module `eip712`
   --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ethers-derive-eip712-2.0.2/src/lib.rs:104:39
    |
104 |     let parsed_fields = match eip712::parse_fields(ast) {
    |                                       ^^^^^^^^^^^^ not found in `eip712`

error[E0277]: the trait bound `EIP712Domain: From<&DeriveInput>` is not satisfied
  --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ethers-derive-eip712-2.0.2/src/lib.rs:86:55
   |
86 |     let domain = match eip712::EIP712Domain::try_from(ast) {
   |                        ------------------------------ ^^^ the trait `From<&DeriveInput>` is not implemented for `EIP712Domain`
   |                        |
   |                        required by a bound introduced by this call
   |
   = note: required for `&DeriveInput` to implement `Into<EIP712Domain>`
   = note: required for `EIP712Domain` to implement `TryFrom<&DeriveInput>`

error[E0277]: the trait bound `proc_macro::TokenStream: From<Infallible>` is not satisfied
  --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ethers-derive-eip712-2.0.2/src/lib.rs:88:44
   |
88 |         Err(e) => return TokenStream::from(e),
   |                          ----------------- ^ the trait `From<Infallible>` is not implemented for `proc_macro::TokenStream`
   |                          |
   |                          required by a bound introduced by this call

error[E0277]: the trait bound `EIP712Domain: From<&DeriveInput>` is not satisfied
  --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/ethers-derive-eip712-2.0.2/src/lib.rs:86:24
   |
86 |     let domain = match eip712::EIP712Domain::try_from(ast) {
   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<&DeriveInput>` is not implemented for `EIP712Domain`
   |
   = note: required for `&DeriveInput` to implement `Into<EIP712Domain>`
   = note: required for `EIP712Domain` to implement `TryFrom<&DeriveInput>`

Some errors have detailed explanations: E0277, E0425.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `ethers-derive-eip712` due to 7 previous errors
warning: build failed, waiting for other jobs to finish...
error: process didn't exit successfully: `/home/runner/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo test --tests --manifest-path /home/runner/work/general-testing/general-testing/Cargo.toml --target-dir /home/runner/work/general-testing/general-testing/target/llvm-cov-target` (exit status: 101)
Error: Process completed with exit code 1.

aasseman commented 1 year ago

Hum this looks like compile errors, I don't see a coverage anywhere. Did you change some of the source code to get these errors? Or does llvm-cov have some kind build issue itself?

pablogmorales commented 1 year ago

Seems to be something with the container image, am checking locally.

ColePBryan commented 1 year ago

Is this most up to date repo? I think this was fixed in #50 .

pablogmorales commented 1 year ago

Fixed locally.

pablogmorales commented 1 year ago

@ColePBryan @aasseman

locally works but not on GH, weird, same repo same code, will check a couple of things and them move along with a different tool do not want to spend much more time on this task which should be something simple.

ColePBryan commented 1 year ago

@pablogmorales are you sure it has #50 ? I don't think it should be using ethers-derive-eip712 anymore, instead it should be using ethers-contract-derive which contains eip712 derive. Could you check the cargo.toml file?

pablogmorales commented 1 year ago

@ColePBryan @aasseman