Skip to content

[WIP] Remove airplane dependency from "test_intercept.js"#1104

Merged
gr2m merged 3 commits intomasterfrom
mbadola/refactor-test_intercept
May 9, 2018
Merged

[WIP] Remove airplane dependency from "test_intercept.js"#1104
gr2m merged 3 commits intomasterfrom
mbadola/refactor-test_intercept

Conversation

@mbad0la
Copy link
Copy Markdown
Contributor

@mbad0la mbad0la commented Mar 17, 2018

Fixes #1103 . Part of #1077

@mbad0la
Copy link
Copy Markdown
Contributor Author

mbad0la commented Mar 17, 2018

Hey @gr2m ,
I have refactored one of tests in this file. Can you take a look at it and let me know if I wrote the test incorrectly?

.get('/abc')
.reply(301, 'served from our mock')
.get('/wont/get/here')
.reply(301, 'served from our mock')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why a 301 response instead of a 200? If you do a 301 you have to set the location header: https://en.wikipedia.org/wiki/HTTP_301

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, I intend to use 304 🙂
It was to differentiate b/w requests which pass through and the ones which match an interceptor

@gr2m
Copy link
Copy Markdown
Member

gr2m commented Mar 17, 2018

It’s quite long, but we can refactor later, one after the other :)

@mbad0la
Copy link
Copy Markdown
Contributor Author

mbad0la commented Mar 17, 2018

I felt so too, but I wanted to keep changes focused and unit in nature, so just removing AIRPLANE dependency for now!

@gr2m gr2m merged commit 5977cdf into master May 9, 2018
@gr2m gr2m deleted the mbadola/refactor-test_intercept branch May 9, 2018 05:38
@lock
Copy link
Copy Markdown

lock bot commented Sep 13, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue and add a reference to this one if it’s related. Thank you!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants