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

Add mock https server and redo start of browser tests for this purpose. #1342

Merged
merged 3 commits into from Jan 8, 2015

Conversation

eiriksm
Copy link
Member

@eiriksm eiriksm commented Jan 7, 2015

This is a suggestion ref #1308.

Using a mock https server, and starting karma as a child process. Let me know if I should change something around :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) when pulling d771671 on eiriksm:bug/fix-browser-tests into 801947f on request:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d0737c8 on eiriksm:bug/fix-browser-tests into 801947f on request:master.

@nylen
Copy link
Member

nylen commented Jan 7, 2015

Excellent work, thank you! I'll merge this tomorrow if no one objects or beats me to it.

For some context, we were getting 403 Forbidden from the GitHub API when running on Travis, which caused the build in #1341 to fail.

@eiriksm
Copy link
Member Author

eiriksm commented Jan 7, 2015

Sounds good :)

And of course it was about time we had tests not relying on 3rd party APIs.

That being said, when trying to get the tests to run with ignoring the self signed cert I noticed that the error message is not so descriptive if you get an "aborted" request like that (meaning the request never really gets a status code) in the browser.

I'll see if I can track it down and/or open an issue about it one of these days

@eiriksm
Copy link
Member Author

eiriksm commented Jan 8, 2015

Ah, sorry. I did not see that PR or take part in that discussion, but I see it now. And makes sense.

If it ends up being defined in one of the test files, it should probably be pulled from there later. As of now (since this is not in master yet) I just changed the port.

@eiriksm
Copy link
Member Author

eiriksm commented Jan 8, 2015

For reference (for me and others), this is the issue in question: #1337

Nice issue number :p

@simov
Copy link
Member

simov commented Jan 8, 2015

Actually 1337 is a good port number too :)

@nylen
Copy link
Member

nylen commented Jan 8, 2015

I want to go ahead and get this in, thanks again @eiriksm!

nylen added a commit that referenced this pull request Jan 8, 2015
Add mock https server and redo start of browser tests for this purpose.
@nylen nylen merged commit f09c68d into request:master Jan 8, 2015
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

4 participants