redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.32k stars 993 forks source link

Add missing units tests to CLI package (and standardize fixtures/mocks for existing) #682

Open thedavidprice opened 4 years ago

thedavidprice commented 4 years ago

Existing good test coverage:

Questions:

Commands/Options needing test coverage (or test improvements):

Not clear in all cases above how to write effective tests.

barakcodes commented 4 years ago

Hi there! @thedavidprice . I am a new contributor, can I take this on?

thedavidprice commented 4 years ago

Hi @barakcodes! If you're willing, this would be a huge help. I just apologize in advance for the current state of mess. 😬

Because there are some general questions we have around CLI testing in general and one aspect of this would be to improve CLI test structure+process, here's my suggestion for a plan of attack:

  1. After you've looked at the code a bit, start with outlining a plan of approach (could be here in this Issue or via a Draft PR). You might need to try some strikes or experiments. Knowing there is some nuanced complexity here, I'd hate to see you spend time on a solution and then have to try it another way.
    • it might be best to break this into smaller PRs per command or set as you feel is manageable
    • these tests don't need to cover every possible command+option, but the more the better. In general, we're looking for an efficient way to be able to trust them, and I'm not sure how that's going to work across the board. E.g. see this current issue with yarn rw db save on Windows -- https://github.com/redwoodjs/redwood/issues/575
  2. Loop in @RobertBroersma as a sounding board for questions and ideas -- he's been focused on the Jest test tooling for Redwood Apps, but could likely help out here if/when needed.
  3. We should also document this work once you're done to make it easier for others to manage current tests and add tests as new commands are created. Both @jtoar and myself can help with the documentation itself, so don't feel like you have to take that part on. (The document that we'll update specifically is the CLI package README.md.)

Some notes and background:


Anything missing or further questions at this time? And, assuming I didn't scare you off, are you still interested? (I hope so! But also know we can break this down into smaller steps as needed if needed 🚀)

thedavidprice commented 4 years ago

FYI: there's a new document coming for CLI and also explains the Generator tests https://github.com/redwoodjs/redwood/pull/663

barakcodes commented 4 years ago

Hey @thedavidprice ! I'm still here. Got a bit occupied I'll get to it and run by you my plan of approach and we can discuss it. I'm very excited about it

thedavidprice commented 4 years ago

@barakcodes sounds fantastic. And grateful in advance for the help 💯