Fix rw test watch logic

[cannikin]

The logic that would set the proper watch flag based on what kind of repo this is would ALWAYS run, regardless of whether the user set the --watch flags or not when running yarn rw test. Yargs was defaulting those flags to false so let's respect that choice and only run the watch logic check if the user actually wants to watch!

We reversed this decision, see discussion! There was still a bug (you couldn't not start Jest without watching) but in addition we also properly default watch to true and user can turn off with --no-watch.

/cc @thedavidprice @RobertBroersma @peterp

[thedavidprice]

+1 default to --watch disabled

[RobertBroersma]

In my experience 99% of the time I run tests locally I want to watch them, that's how this piece came into existence.

This is also the default in CRA: https://create-react-app.dev/docs/running-tests/#command-line-interface

I think defaulting watch to true would solve both our problems?

[peterp]

Just want to chime in here and say that making it watch by default was something that me and @RobertBroersma chatted about, and I think he's right, making it watch by default is usually what I want.

I think the question is does --no-watch work?

[RobertBroersma]

@peterp I think that worked for a brief moment CRA: https://stackoverflow.com/a/39726625/2211961

[peterp]

Ah, what I mean is that on our side if we type yarn rw test --no-watch can we disable the watch command? If an arg is a boolean in yargs then you can often just write --no-<boolean-args>

[RobertBroersma]

Really? TIL

[thedavidprice]

Yes, you can disable a Boolean in Yargs with --no-[option].

Just as Jest itself doesn't default to watch, I don't think this command should either as I'm not convinced that's the expected behavior or starting place for most users. (RC, would be interested to hear your thoughts re: 2torial flow, if it matters one way or the other.) Also:

• It's more intuitive to "enable" features and options with flags than it is to disable
• If it is decided that this will be enabled by default, we should then update the description to explain to users how to disable it with --no-watch. That's not provided in the --help output and doesn't seem to be universally known.

Lastly, we definitely need to be able to disable watch for CI use cases.

[peterp]

I'm not convinced that's the expected behavior or starting place for most users.

The majority of time when I'm running tests it's because I'm writing tests and when I'm writing tests this is the expected behaviour.

[cannikin]

For me personally, when I'm in Ruby mode, I have a keyboard shortcut that runs the current test file and another that runs the current test (whichever one the cursor is in) and that's my usual flow when I'm writing tests. The auto-run functionality in Ruby never felt quite ideal. I also find it distracting with all the runs happening which are obviously going to fail as I'm still writing the tests but saving in the meantime.

David has a great point that if it's not the default for Jest then maybe it shouldn't be the default here either?

The downside to that, which I found since #1414, is that yarn rw db up runs at the start of the test suite (watching or not) which significantly slows startup time. Having to go through that before each and every run is tough. :( Maybe we could do something like hash the content of prisma.schema and store that in /.redwood and only run yarn rw db up on startup if hashing the schema at the start doesn't match the one in /.redwood from the previous run?

[peterp]

I hear you, and Jest in watch mode has this awesome menu:

Watch Usage
 › Press f to run only failed tests.
 › Press o to only run tests related to changed files.
 › Press p to filter by a filename regex pattern.
 › Press t to filter by a test name regex pattern.
 › Press q to quit watch mode.
 › Press Enter to trigger a test run.

You can filter by failing, regex, or filename. I actually use wallaby now, and usually just do something like test.only() when I'm writing a test.

👍 on the db stuff, I actually stopped using yarn rw test because it was too slow for me.

[thedavidprice]

I actually use wallaby now, and usually just do something like test.only() when I'm writing a test.

^^ @peterp if and when there's time, I'm very interested to see you workflow here

on the db stuff, I actually stopped using yarn rw test because it was too slow for me.

^^ are you using wallaby here as well? If not, curious what you've replaced this with and if we should add to Redwood or improved the Redwood DB tests to be more performant?

[cannikin]

Seems like people who actually use this aren't loving this change—I'm gonna close this for now!

[cannikin]

hahaha Here we go

[thedavidprice]

@cannikin This PR does two things:

• it resolves a bug with the Watch option
• in fixing the bug, it sets the default Watch state to disabled

We need to fix the bug.

If it is decided that this will be enabled by default, we should then update the description to explain to users how to disable it with --no-watch. That's not provided in the --help output and doesn't seem to be universally known.

CLI Docs should be updated as well.

Happy to help with any/all of the above.

[cannikin]

Okay coordinate with @peterp and @RobertBroersma to decide which one, but I'm not comfortable merging as it currently is with them both disliking the behavior this change introduces (disabling watch by default).

[peterp]

I started using yarn rw test again today after @RobertBroersma explained the awesomeness to me!

[peterp]

@cannikin Just a reminder of our conversation on the zoom call: So we're going to keep this, but we'll flip the watch and watchAll booleans to true (watch by default).

And then I guess we need to make sure that we can do yarn rw test --no-watch

I wonder if we need to provide both the watch and watchAll flags here? @RobertBroersma, could we simplify this and just introduce a --watch flag that does --watchAll if you're not using Git?

[RobertBroersma]

I wonder if we need to provide both the watch and watchAll flags here? @RobertBroersma, could we simplify this and just introduce a --watch flag that does --watchAll if you're not using Git?

I think that would work. Especially since running with --watch and pressing a is pretty much the same as --watchAll

[cannikin]

Okay folks let me know how this update looks:

• watch is defaulted to true
• Neither --watch nor --watchAll are added to the list of underlying Jest arguments unless watch is true (and then it will add --watch or --watchAll depending on whether you are in a git/mecurial repo or not)

[RobertBroersma]

Looks great!