search

learn-co / learn-test

⚠️ This gem is no longer maintained & its web features are no longer available
MIT License
3 stars 29 forks source link

Move reporter requirements to the project under test for Mocha strategy #25

Closed pletcher closed 8 years ago

pletcher commented 8 years ago

This strategy was the easiest to move, so I started here. It's coordinated with the addition of https://github.com/learn-co-curriculum/curriculum-guide-writing-tests to the Curriculum Guide (already in) and https://github.com/learn-co-curriculum/node-js-npm-lab.

The main issue that I can see is that this change will break older Mocha labs. While we're migrating those labs to conform to these new requirements, I've kept some backwards-compatibility here.

loganhasson commented 8 years ago

nice! before we merge this in, let's have kaitlin take a QA pass at it, but then i'm all for it

pletcher commented 8 years ago

@kvignali, if there's bandwidth, I would love to get this (or a fixed/improved version of it) in now that we're working on the JavaScript and Node.js tracks. Thanks!

cc @loganhasson

loganhasson commented 8 years ago

@pletcher bah, sorry i spaced on getting this in. will all our existing curriculum work out of the box with this?

pletcher commented 8 years ago

No worries! Existing curriculum should be unaffected -- I can try to carve out time to verify. The else branch preserves current functionality for labs that don't mention ".results.json" in their test script.

joshrowley commented 8 years ago

seems legit to me 👍

pletcher commented 8 years ago

Hey all, I just added another change -- hopefully it's minor, but happy to put the brakes on if it seems too much.

I added a check for node_modules/ and an npm install run if the directory doesn't exist. This is because a lot of these labs will have some basic version of dependencies that students won't need to finish the lab but will need to run the tests -- basically just a hacky check so that we can install if needed.

loganhasson commented 8 years ago

💥