Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Write tests #1

Open
jack-lewin opened this issue Oct 2, 2017 · 9 comments
Open

Write tests #1

jack-lewin opened this issue Oct 2, 2017 · 9 comments

Comments

@jack-lewin
Copy link
Owner

It would be good to have a few tests to cover basic functionality. I'd prefer to use Mocha or Jest to run the tests - either is fine 👍

Maybe this is a good starting point for tests to write (click to expand)

app.js

  • connect()
    • returns a GitHub API object
    • logs to console.warn if no API key has been provided
  • getIssues()
    • returns a promise
    • resolves with an array in the format [ repo: [ issues ] ]
    • rejects if unsuccessful (maybe we could add some validation to ensure params are received)
  • notify()
    • sends a GET request to the specified URL
  • parseIssue()
    • returns expected values from a GitHub issue object

helpers.js

  • formatTime()
  • timeMinsAgo()
    • get the time (in milliseconds), X minutes ago
@jacquesdev
Copy link
Contributor

I'd love to take this one on if possible? Will probably take me a day or two though.

@jack-lewin
Copy link
Owner Author

@jacquesdev sure thing, that would be great!

@tmarkovic
Copy link

I would love to help out as well!

@jacquesdev
Copy link
Contributor

jacquesdev commented Oct 3, 2017

@tmarkovic - that sounds great to me as well. If it's ok with you - I just want to setup mocha and maybe one test so that it's in place, then you can continue with the rest of the tests, and I can fill any gaps going forward. I just wanted an opportunity to do a mocha implementation from scratch.

@tmarkovic
Copy link

@jacquesdev Just my thought, there's no reason why we can't split the writing of tests as long as a runner and framework is in place! Have you given Jest any thought though?

Looking forward to this either way 👍

@jack-lewin
Copy link
Owner Author

Awesome - thanks @tmarkovic @jacquesdev!

@tmarkovic
Copy link

I've started writing a test for the notify function, though I must admit that I'm not very experienced with testing at all so it might take me some time.

My current approach is to use Nock to mock the HTTP-call. Does it make sense to use a stub for parseIssue? I feel like it would be nice to not couple the test to github specific data but I may very well be wrong here.

Any recommendations on how I should go about constructing the IFFT url before the test?

@jack-lewin
Copy link
Owner Author

Hi @tmarkovic, sorry for the delayed response.

That's a good idea - if you stubparseIssue to return an object with { owner, repo, title, url, labels }, we'll avoid re-testing that function.

If you also declare process.env.MAKER_KEY in the test, we can just make sure we send a GET request to the correct URL. We'll know what the URL is, based on the data we get from parseIssue (which we're stubbing anyway) 🙂

@jack-lewin
Copy link
Owner Author

Hi @jacquesdev @tmarkovic - sorry I haven't followed up sooner on this.

Did either of you get the chance to continue working on this? If so, it would be great to see how it's going 🙂 Happy to lend a hand if you've run into any problems.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants