out-of-cheese-error / gooseberry

A command line utility to generate a knowledge base from Hypothesis annotations
Apache License 2.0
153 stars 9 forks source link

gooseberry sync now produces better output #27

Closed kevloui closed 4 years ago

Ninjani commented 4 years ago

Looks good! Could you run it through cargo fmt?

kevloui commented 4 years ago

Yep! Runs through cargo fmt with no problems for me!

Ninjani commented 4 years ago

There's still the last case to handle, i.e. when nothing changes (see the linked issue) - could be done with an if statement checking if all three are 0.

kevloui commented 4 years ago

I've run the new code through cargo fmt, it builds fine too!

Ninjani commented 4 years ago

Do the tests pass? Here's some instructions on setting that up

kevloui commented 4 years ago

Hmm... now I'm getting this:

running 3 tests
test it_works ... FAILED
test sync ... FAILED
test tag ... FAILED

failures:

---- it_works stdout ----
Error: 
   0: path not found
   1: path not found
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /Users/keverne/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/macros.rs:16:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- sync stdout ----
Error: 
   0: path not found
   1: path not found
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /Users/keverne/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/macros.rs:16:9

---- tag stdout ----
Error: 
   0: path not found
   1: path not found
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /Users/keverne/.rustup/toolchains/stable-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/macros.rs:16:9

failures:
    it_works
    sync
    tag

test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--test cli'

I'll see if it happens without the changes too.

Ninjani commented 4 years ago

Haven't encountered that error before but I noticed that the tag test sometimes failed due to querying the Hypothesis API before the changes were reflected - I've added some delays to this on the main branch.

I am fine with running tests locally for now if you couldn't get them to work but in the current state the PR wouldn't pass the tests since there are still some occurrences of "Added 2 notes" and "Updated 2 notes" that haven't been changed. Could you look into changing those?

Thanks!

kevloui commented 4 years ago

I’ll give these a look tomorrow, hopefully we can solve it.

kevloui commented 4 years ago

Haven't encountered that error before but I noticed that the tag test sometimes failed due to querying the Hypothesis API before the changes were reflected - I've added some delays to this on the main branch.

I am fine with running tests locally for now if you couldn't get them to work but in the current state the PR wouldn't pass the tests since there are still some occurrences of "Added 2 notes" and "Updated 2 notes" that haven't been changed. Could you look into changing those?

Thanks!

Are the occurences of "Added 2 notes" and "Updated 2 notes" in gooseberry sync? Or are you talking about the tests? Also, the error is still happening for me, still looking into the issue!

kevloui commented 4 years ago

Ahh I realised I was adding the wrong cli.rs to the commit. The new cli.rs in /tests/ should be the right one.

Ninjani commented 4 years ago

Are you sure you've followed the instructions for setting up tests from here? It's likely that you're missing the .env file causing the tests to fail. The tests run fine for me now so I'll accept the PR.

kevloui commented 4 years ago

Ahh I just realised that I lost the .env file when I re-cloned the repo. I’ll make a copy outside of the repo now!