Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spike: Convert try-o-rama tests to Sweettest [WIP] #92

Closed
wants to merge 7 commits into from

Conversation

harlantwood
Copy link
Collaborator

@harlantwood harlantwood commented Mar 20, 2023

Making this initial spike to open a discussion -- I find try-o-rama tests to be:

  • flaky, especially on CI (see screenshot)
  • require sleeps (making them slow)
  • often give no usable stacktrace, even into the test lines -- ie all lines are within tape in node_modules

By contrast I find Sweettest to be:

  • rock solid, never flaky
  • no sleeps required -- and faster overall even with compilation times
  • good stacktraces into the line that failed in test code

Oh and of course you can directly use your Rust structs as types! Huge benefit.

Here's an apples to apples side by side conversion of the first test, which I find much more pleasant and readable and maintainable in Rust:

Screen Shot 2023-03-19 at 3 02 47 PM

I'll post some screenshots on the other points in comments.

@harlantwood
Copy link
Collaborator Author

harlantwood commented Mar 20, 2023

Try-o-rama tests hanging, running for 5 hours; sometimes passing on mac runner but failing on ubuntu:

Screen Shot 2023-03-19 at 2 34 03 PM

A day later, the opposite: Ubuntu passes and MacOS hangs:

A0634A8C-161F-49AD-91F0-74BCD379D235

@harlantwood
Copy link
Collaborator Author

Usable stacktrace in sweettest:

Screen Shot 2023-03-19 at 2 16 16 PM

Unusable stacktrace in Try-o-rama -- something went wrong, but no lines are even in our test file!

Screen Shot 2023-03-18 at 3 23 13 PM

@harlantwood harlantwood changed the title Convert try-o-rama testst to Sweettest 💜 [WIP] Spike: Convert try-o-rama tests to Sweettest [WIP] Mar 20, 2023
@jost-s
Copy link
Collaborator

jost-s commented Mar 20, 2023

We could convert those tests to sweettest. Regarding your comments:

  1. I'd rather like to find out where the flakiness comes from. Locally I don't experience flakiness with Tryorama, so there must be something going on in the pipeline.
  2. Sleeps, yeah, because it's testing over the wire and doesn't use conductor methods directly, and uses a full conductor. It's a different kind of test, but for the tests we're doing here which are mainly of unit test nature, the sweettest tests would be fine.
  3. The stack traces are rather an issue with how the tape test runner behaves than with Tryorama. It doesn't catch errors for you and logs them. With Jest for example this doesn't happen. In Tryorama there's the function runScenario which wraps your test with a try..catch block. We can convert the tests to use that function.

An important argument to keep using Tryorama (even a general argument) is that you're testing the actual JavaScript/TypeScript API that the web app is using. With sweettest you're using the Rust types of your app that Holochain uses, which is handy. But the web app is the one that consumes the zome functions, and Tryorama is a way to test your TS types and functions in that way.

@mattyg
Copy link
Collaborator

mattyg commented Mar 20, 2023

Yeah I personally lean on the side of "your tests should follow as close to the actual paths that your code follows as possible". And if you have limited resources, focus on E2E tests that touch the broadest paths, even at the cost of longer test suite run time.

I opened a ticket in tryorama repo for one option to address the 'sleep' thing (holochain/tryorama#167) .

It seems like part of the slow ci is also the time to spin up a nix environment in the test runner. Figuring out how to get that running reliably seems pretty important for the whole ecosystem.

I quite like the output format of the node test runner Ava (https://github.com/avajs/ava). Maybe switching to that would give more usable output (I agree that the tape ones are not very helpful).

@harlantwood
Copy link
Collaborator Author

harlantwood commented Mar 22, 2023

I definitely agree that there is advantage in using the TypeScript types that our frontend uses in the tests. @jost-s said it well:

An important argument to keep using Tryorama (even a general argument) is that you're testing the actual JavaScript/TypeScript API that the web app is using. With sweettest you're using the Rust types of your app that Holochain uses, which is handy. But the web app is the one that consumes the zome functions, and Tryorama is a way to test your TS types and functions in that way.

To me this is the only compelling advantage of Tryorama.

@mattyg while I agree in principle with this idea:

Yeah I personally lean on the side of "your tests should follow as close to the actual paths that your code follows as possible". And if you have limited resources, focus on E2E tests that touch the broadest paths, even at the cost of longer test suite run time.

...I would say that other than the typescript types, all we're testing in these particular tests (beyond the zome function unit tests at the core) is the HC JS client / conductor setup itself -- which has its own tests, no need to test them in our happ tests.

So what about the typescript types?

Well, best case is that we generate them from the rust structs -- ie the input types for our zome's hdk_extern funtions.

@guillemcordoba has done some experimenting with https://crates.io/crates/ts-rs -- if we could get this working, and generate the TS types from the Rust types, then would folks be supportive of using Sweettest for these tests -- which, as Jost mentioned, amount to zome function unit tests?

@harlantwood
Copy link
Collaborator Author

Closing after chatting with @mattyg — feel free to reopen in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants