Skip to content
This repository was archived by the owner on Jul 14, 2020. It is now read-only.

More run tests#20

Merged
wwilsman merged 17 commits intomasterfrom
ww/run/more-tests
Sep 14, 2018
Merged

More run tests#20
wwilsman merged 17 commits intomasterfrom
ww/run/more-tests

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

Purpose

Adds unit tests and docs for most of the remaining run pieces.

Approach

  • Moved around test helpers and utils so it's a bit more organized
  • Cleaned up the logging
  • Added tests and docs for the serve and adapter plugins
  • Added tests and docs for the various manager classes
  • Switched to using sinon for spies and stubs instead of hand-rolled fakes
  • While writing tests for these pieces, the code was tightened up a bit (made easier to test and updated to desired behavior).
  • Updated the coordinator to reflect changed pieces
  • Changed the CLI options passed to run

The new CLI options are meant to remove the dot-notation. While being able to specify objects as arguments via the CLI is pretty cool, it has it's limitations. In most places we were using the dot-notation we needed to have special logic in place in order to properly merge options from the opts file. This special logic also included a hack involving specifying the type as an array which made the help output untrue and confusing.

I probably should have split this up into multiple PRs, but it's been sitting around for a few days so I just wanted to get everything in (since it's mostly tests).

Todo

  • There are still no unit tests for the coordinator class, or individual browser launchers. Not sure how we would test the launcher classes without actually launching the browsers.
  • Instead of making unit tests for the adapters and client code, I think we could test them using the CLI itself, resulting in acceptance testing the CLI while also testing the individual adapters.

@wwilsman wwilsman requested a review from Robdel12 September 14, 2018 15:32
Copy link
Copy Markdown
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to the opts are nice 👍 lines back up with my talks haha

@wwilsman wwilsman merged commit efbf91a into master Sep 14, 2018
@wwilsman wwilsman deleted the ww/run/more-tests branch September 14, 2018 16:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants