Nock-1077 Remove airplane mode by jlilja · Pull Request #1209 · nock/nock

jlilja · 2018-09-07T15:48:48Z

Reference to #1077

Updated tests to use its own https server.

Query now also respects return value false. Why this happens during my rewrite I cannot understand or see. I think however passing functions in tests should be tested in file test_complex_querystrings.js.

Do we know the expected behavior of passing false in a function in query? If we do and it doesn't match the expected behavior of the second test, that should call for a new issue/pr.

…turn value false.

gr2m · 2018-09-09T09:06:18Z

lib/interceptor.js

+    if (queries && _.isFunction(queries) && queries() === false) {
+      this.queries = {}
+      return this
+    }


Can you make a separate PR for the code change and only do tests? It's harder to review that way

Sure!

Also, care to guess the expected behavior when function returns false in the query function? Just so I don't misinterpret the results I'm getting.

I don’t know 🤷‍♀️ Can you open a separate issue/PR for discussion?

Yeah. Once this test is fixed I'll work on it. :)

@jlilja Any time to work on this? :)

paulmelnikow · 2018-12-16T22:05:10Z

tests/test_https_allowunmocked.js

+    request(options, (error, response, body) => {
+      test.true((error === null), 'should be no error')
+      test.true(typeof body !== 'undefined', 'body should not be undefined')
+      test.true(body.length !== 0, 'body should not be empty')


It looks like this is intended to fall through to the real server. Could you add assertions that match the body and/or status code to what the server returns?

gr2m · 2018-12-16T23:39:59Z

@jlilja looook: #1209 let’s finish this up 🙌

gr2m · 2018-12-20T21:13:13Z

@jlilja mind if we finish this up? It's the last bit to get rid of the AIRPLANE mode and it’s starting to block

paulmelnikow · 2019-01-02T16:48:13Z

@gr2m do you want to wrap this one up?

gr2m · 2019-01-02T16:55:00Z

I’ll be busy until Jan 9th, I can look into it afterwards. Feel free to tackle it yourself if you like

jlilja · 2019-01-03T13:46:49Z

Terribly sorry, I've been away for holidays! If you haven't already taken over this PR, I will fix this today. @paulmelnikow have you started working on this?

paulmelnikow · 2019-01-03T14:02:43Z

Please do! 😁

We're targeting the beta branch now so probably best to start a new branch and manually pick over the changes. You'll want to set up Prettier to handle formatting as well (or run npm run prettier if you don't want to set it up).

jlilja · 2019-01-03T14:16:22Z

Sweet! Been using prettier since before, so no problem. Will the beta branch be a big merge with all of the tests updated? Or is this a branch that'll stay for future changes as well?

gr2m · 2019-01-03T16:53:52Z

We will eventually merge beta into master which will then automatically release the new breaking version. After that we probably make master the main branch again until we want to work with pre-releases again :)

paulmelnikow · 2019-01-05T20:40:43Z

@jlilja How'd you make out?

…t not refuse any other path than index.

jlilja · 2019-01-05T22:23:07Z

I've made a fix for my code to do the check you asked about. However I couldn't make it work with the request package. I instead tried to do the same logic however with axios - and with axios it works just as I want.

Now this probably raises another question as to why axios works and request fails.

Depending on what you think I can probably merge it into beta or I'll go back to try and troubleshoot why it isn't working with request.

jlilja · 2019-01-05T22:28:38Z

I just saw that you're converting to the got package in other commits. If I switch from axios to got (and fix that linting travis notices), are we good? :)

jlilja · 2019-01-06T09:40:38Z

I've been applying linting fixes from the rules in the beta branch but it seems to still be complaining about my code in Travis, however not locally. Could someone have a look?

paulmelnikow · 2019-01-06T10:19:28Z

One thing you could try is deleting node_modules and running npm install again. We have a lockfile now. Also, merge beta into this branch (or rebase this onto beta) if you haven't already.

Oh! I see that's the problem. Some updates to the lint config made in the beta branch are necessary for the tests to pass.

paulmelnikow · 2019-01-06T10:26:05Z

tests/test_https_allowunmocked.js

+    const { body } = await got(`${url}/otherpath`)
+    t.true(JSON.parse(body).status === 'default')
+  } catch (error) {
+    console.warn(error)


I think these try/catch should be removed so those conditions will just throw. What do you think?

That is probably better. Didn't think of the error handling while in a test.

paulmelnikow · 2019-01-06T10:26:21Z

tests/test_https_allowunmocked.js

+    console.warn(error)
+  }
+
+  t.end()


t.end() will happen automatically on tests with async.

Didn't know that. I'll fix it.

…into Nock-1077-allow_unmocked_js

paulmelnikow · 2019-01-06T13:29:50Z

tests/test_https_allowunmocked.js

-  nock('https://www.google.com/', { allowUnmocked: true })
-    .get('/pathneverhit')
-    .reply(200, { foo: 'bar' })
+process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = 0


Can this be done with { rejectUnauthorized: true } passed to got instead?

This i so confusing. While trying to change, the test appears to be working without the tls boolean. Could have been fixed I set the port explicitly to 3000 instead of letting https.createServer assign a random port. I'll push and see what Travis says as well.

No. Stupid me. Was changing in the wrong repo and branch...

jlilja · 2019-01-06T14:06:12Z

I'm still a bit confused if I've merged/rebased correctly. Let me know if I can do anything differently from here on.

paulmelnikow · 2019-01-06T15:05:55Z

It looks good! In the future if you start with the beta branch things should be easier. 😁

gr2m · 2019-01-06T16:30:53Z

OMG! Did we just hit our first big milestone?! A celebration is in order /cc @RichardLitt

I see #1367 is the last bit to close #1077 :)

jlilja · 2019-01-06T17:53:41Z

This is so exciting! This should be posted in #1268 😄

RichardLitt · 2019-01-11T09:48:49Z

Woo! Well done, @jlilja!

nockbot · 2019-01-19T03:13:40Z

🎉 This PR is included in version 11.0.0-beta.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

nockbot · 2019-08-13T00:03:22Z

🎉 This PR is included in version 11.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

jlilja added 3 commits September 7, 2018 17:33

Updated tests to use its own https server. query now also respects re…

903763e

…turn value false.

Linting fix.

2887b7f

Update es6 syntax

2d0b7ec

weekly-digest bot mentioned this pull request Sep 9, 2018

Weekly Digest (2 September, 2018 - 9 September, 2018) #1211

Closed

gr2m reviewed Sep 9, 2018

View reviewed changes

RichardLitt mentioned this pull request Sep 14, 2018

Nock Office Hours #1202

Closed

weekly-digest bot mentioned this pull request Sep 16, 2018

Weekly Digest (9 September, 2018 - 16 September, 2018) #1226

Closed

weekly-digest bot mentioned this pull request Oct 14, 2018

Weekly Digest (7 October, 2018 - 14 October, 2018) #1243

Closed

gr2m mentioned this pull request Dec 14, 2018

Remove AIRPLANE mode for testing - make all tests run offline #1077

Closed

42 tasks

jlilja added 5 commits December 14, 2018 21:44

Makes tests work again.

c5439eb

Merging from master

33f761f

Removing

726c117

Removing merge seperators

3c4ca54

Added newline at end of file

b568de3

weekly-digest bot mentioned this pull request Dec 16, 2018

Weekly Digest (9 December, 2018 - 16 December, 2018) #1282

Closed

paulmelnikow reviewed Dec 16, 2018

View reviewed changes

paulmelnikow mentioned this pull request Dec 19, 2018

Code formatting #1300

Closed

weekly-digest bot mentioned this pull request Dec 23, 2018

Weekly Digest (16 December, 2018 - 23 December, 2018) #1331

Closed

Reworked the server response and switched to axios to make the reques…

ce09f3c

…t not refuse any other path than index.

jlilja added 2 commits January 6, 2019 10:26

Added eslint linting fixes

ae81696

Possible fix for eslint error

2d8c28b

paulmelnikow changed the base branch from master to beta January 6, 2019 10:20

paulmelnikow added 2 commits January 6, 2019 05:22

Merge branch 'beta' into Nock-1077-allow_unmocked_js

2ae4294

Rm unnecessary prod dependency

e8369f5

paulmelnikow reviewed Jan 6, 2019

View reviewed changes

jlilja added 2 commits January 6, 2019 13:22

Removing error handling and t.end function within async functions

141b171

Merge branch 'Nock-1077-allow_unmocked_js' of github.com:jlilja/nock …

7194cf6

…into Nock-1077-allow_unmocked_js

paulmelnikow reviewed Jan 6, 2019

View reviewed changes

jlilja added 2 commits January 6, 2019 14:55

Removed node self signed cert check and added in got options

2805999

Remove debugging consoles

9365c6c

paulmelnikow approved these changes Jan 6, 2019

View reviewed changes

paulmelnikow merged commit 26954a7 into nock:beta Jan 6, 2019

nockbot added the released on @beta label Jan 19, 2019

nockbot added the released label Aug 13, 2019

gr2m pushed a commit that referenced this pull request Sep 4, 2019

test(test_https_allowunmocked) Remove airplane mode (#1209)

97f0eff

gr2m pushed a commit that referenced this pull request Sep 4, 2019

test(test_https_allowunmocked) Remove airplane mode (#1209)

ec8cd96

nockbot mentioned this pull request Sep 4, 2019

The automated release is failing 🚨 #1704

Closed

gr2m pushed a commit that referenced this pull request Sep 5, 2019

test(test_https_allowunmocked) Remove airplane mode (#1209)

ebe33bc

juninmd pushed a commit to juninmd/nock that referenced this pull request Mar 21, 2026

test(test_https_allowunmocked) Remove airplane mode (nock#1209)

93bce2b

Uh oh!

Conversation

jlilja commented Sep 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gr2m commented Dec 16, 2018

Uh oh!

gr2m commented Dec 20, 2018

Uh oh!

paulmelnikow commented Jan 2, 2019

Uh oh!

gr2m commented Jan 2, 2019

Uh oh!

jlilja commented Jan 3, 2019

Uh oh!

paulmelnikow commented Jan 3, 2019

Uh oh!

jlilja commented Jan 3, 2019

Uh oh!

gr2m commented Jan 3, 2019

Uh oh!

paulmelnikow commented Jan 5, 2019

Uh oh!

jlilja commented Jan 5, 2019

Uh oh!

jlilja commented Jan 5, 2019

Uh oh!

jlilja commented Jan 6, 2019

Uh oh!

paulmelnikow commented Jan 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlilja commented Jan 6, 2019

Uh oh!

paulmelnikow commented Jan 6, 2019

Uh oh!

gr2m commented Jan 6, 2019

Uh oh!

jlilja commented Jan 6, 2019

Uh oh!

RichardLitt commented Jan 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nockbot commented Jan 19, 2019

Uh oh!

nockbot commented Aug 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

paulmelnikow commented Jan 6, 2019 •

edited

Loading

RichardLitt commented Jan 11, 2019 •

edited

Loading