[WIP] A Proposal For Testing Things #180

Open
wants to merge 13 commits into
from

Projects

None yet

2 participants

@shiftkey
Contributor
shiftkey commented Jan 7, 2017

As we talked about in #178 (comment), it'd be nice to be able to run the verify tests automagically. This PR is my attempt to tackle the problem, which required some refactoring of how the verify scripts work.

Current Limitations

A couple of issues that I've had to work through:

  • verify scripts touch the DOM as part of rendering the results - I've attempted to move this code back up to the challenge script, but it's only done for one verify script so far (the repository tests).
  • some verify scripts are written as if they are blocking but due to using exec they need to be asynchronous - we need a way to inspect the results, so I've introduced a callback that takes an array of results. Maybe you have other ideas in mind.

How This Works

We're using tape to run scripts (I'm more familiar with mocha, so I'm open to other ways to get the best out of the tape API), but here's a simple test:

test('verifies the hello world repository', function (t) {
  var folder = helper.extractFixture('hello-world')
  verify(folder, function (result) {
    var expected = 'This is a Git repository!'
    var first = result[0]

    t.ok(first.result)
    t.equal(expected, first.message)
    t.end()
  })
})

For scenarios where we need the repository to be in a certain state, we can splat the folders onto disk under the tests/fixtures folder - and then extract them to a temporary folder when running the tests. To get this working, you can just rename the .git folder to _git and the repository state is then version-controllable.

The other notable change here is that we have a callback to poke at the results from the verify script (because exec is asynchronous), to ensure we're getting the expected results. I'm using the same shape of "message is a string" and "result is a boolean" that the existing verify scripts use.

To get this working in a test harness, I needed to move the code for addToList and challengeIncomplete/markChallengeCompleted out from the verify script to where the verify method is invoked. A partial implementation of this in challenge.js works for the repository script, but it definitely needs more work and testing as I might not be understanding it correctly.

What's Left To Do?

A rough list:

  • get feedback on this approach
  • sketch out the changes to test a path-less script, update challenge.js to handle this
  • propose a way to test request invocations (nock? something else?)
  • ???
@jlord
Owner
jlord commented Jan 13, 2017 edited

verify scripts touch the DOM as part of rendering the results

Yeah, this is something I've wanted to improve; to separate these concerns. Which is probably a big enough project in itself to be a different PR and a thing to figure out before this?

I haven't gone through all this with a fine tooth comb but in general I feel like it's what I've been thinking! 👍

I have a couple things left to clean up, cause I don't think master is quite stable right now but then I'll pick up from what you've started here in moving out the DOM touching stuff and I'll get a PR going for that.

(Might also have to pause and check into some possibly borked jlord/reporobot stuff too that I noticed tonight 😅 )

@shiftkey
Contributor

🆒

Which is probably a big enough project in itself to be a different PR and a thing to figure out before this?

Up to you - if you feel confident enough to do that refactoring before introducing tests then 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment