Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(test): mock all request responses #376

Merged
merged 8 commits into from Oct 13, 2017

Conversation

koddsson
Copy link
Member

This patch adds mocked responses to all the requests made in the tests.

Here's how I generated the data in mock-data.json using nock:

diff --git a/test/integration/endpoints.js b/test/integration/endpoints.js
index 4ab166c..604de5e 100644
--- a/test/integration/endpoints.js
+++ b/test/integration/endpoints.js
@@ -4,10 +4,28 @@ process.env.INTEGRATION = true
 import fs from 'fs'
 import path from 'path'
 import fileModule from 'file'
+import nock from 'nock'
 
 const testDir = 'tests'
 const testFileName = 'integration_test.js'
 
+before(() => {
+  nock.recorder.rec({
+    output_objects: true,
+    dont_print: true,
+  })
+})
+
+after((done) => {
+  const nockCallObjects = nock.recorder.play()
+  fs.writeFile('./mock-data.json', JSON.stringify(nockCallObjects), (error) => {
+    if (error) {
+      console.error(error)
+    }
+    done()
+  })
+})
+
 describe('endpoint', () => {
   it('should load the server and set everything up properly', (done) => {
     const app = require(`${process.cwd()}/server`)

This means that our tests don't make any requests which has been a problem in the past. This enables us to mock future endpoints from the beginning and maybe we can start moving the mocked data into the tests and delete the json file in the future.

@koddsson
Copy link
Member Author

This failure actually makes sense since the weather API test is failing on master as well 😆

@koddsson
Copy link
Member Author

It looks like the weather site is just broken right now so the failure makes sense at this point of time

image

But this is the kind of issues we get rid of by mocking the website responses like I'm doing in this PR. The fact that this PR and master is blocked on something that we can't change is pretty bad.

@koddsson
Copy link
Member Author

koddsson commented Oct 11, 2017

Regenerated the mock json data in a more human readable form so that diffs will be better in the future and added a way to easily re-record mock data.

The upstream data source is currently broken so we're disablign the test
for now.
@koddsson
Copy link
Member Author

The CI build is running in ~30sec with linting and testing running in ~5sec each.

image

Copy link
Member

@MiniGod MiniGod left a comment

Choose a reason for hiding this comment

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

This is amazing! Could you add something to the readme explaining how to update the mock-data?

@koddsson
Copy link
Member Author

@MiniGod How's this?

@koddsson
Copy link
Member Author

I don't wanna tout my own horn but it's been really fun working in this PR with the really fast and consistent tests 🙏

@kristjanmik
Copy link
Member

This is AMAZING! You are not tooting your horn at all, we will have to call Gudni, the pineapple president to give you one of his fálkaorðas

@MiniGod
Copy link
Member

MiniGod commented Oct 13, 2017

Great note about how to do the mock data for new endpoints. LGTM now.

I love this PR so much. I and @kristjanmik were talking about apis.is last weekend, and how much it sucks that the tests fail if the sources are not up, and hence the automatic deploy fails too. This is seriously such an amazing PR! Kudos to @koddsson

all my thumbs are up

@koddsson koddsson merged commit 848ba06 into apis-is:master Oct 13, 2017
@koddsson koddsson deleted the mock-requests branch October 13, 2017 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants