octokit / octokit.js

The all-batteries-included GitHub SDK for Browsers, Node.js, and Deno.
MIT License
6.86k stars 1k forks source link

make tests easier to run #263

Closed jamestalmage closed 8 years ago

jamestalmage commented 9 years ago

mock out the server for most tests

Really we just need to make sure that a given set of inputs is going to generate the correct request. We should just be able to mock out http and https using proxyquire or similar.

make it easier to set up the auth tokens for existing tests

All the tests should load the auth token from an external file (/github.auth.json), which we add to the .gitignore list so it doesn't get committed.

UPDATE: removed suggestion that would not have worked

We should store the auth token in plain text. Github automatically detects auth tokens in commits, disables the compromised token, and sends the user a warning e-mail.

employ continuous integration

Travis seems the easiest choice here. We could save a token for travis use as hidden an environment variable. That won't work for PR's (travis will not set hidden environment variables for PR's for security reasons), so we just skip e2e tests for PR's.

move tests to their own directory, away from generated code

I prefer placing generated code inside a build or dist directory, and tests inside test. This makes it easier for new contributors to figure out what's happening and where to get started. At first glance I thought this project was untested, which was not a great first impression. I was happy to find out my initial assumption was wrong, but I'm sure others are making the same mistake.

kaizensoze commented 8 years ago

If interested, I have a fork in which I've separated the tests into a test folder and the examples/tests use a test_auth.json file under gitignore: kaizensoze/github4

kaizensoze commented 8 years ago

The kaizensoze/github4 fork has been merged in so it's just a matter of adding a testAuth.json with a token and running npm test. Getting all the tests to pass on the other hand is a different story and another issue. It should really be mocked out.