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 tests for offline mode Web request blocking #103

Merged
merged 2 commits into from
Jan 5, 2015
Merged

Add tests for offline mode Web request blocking #103

merged 2 commits into from
Jan 5, 2015

Conversation

earldouglas
Copy link
Contributor

This revealed a couple of issues with the way we
were enabling offline mode in our tests, the way
we were handling offline requests to be blocked,
and the way we respond to blocked requests. All
have been fixed and are now under test.

This revealed a couple of issues with the way we
were enabling offline mode in our tests, the way
we were handling offline requests to be blocked,
and the way we respond to blocked requests.  All
have been fixed and are now under test.
@earldouglas earldouglas changed the title Add tests for offline mode Web request blocking Add/refactor tests for offline mode Web request blocking Jan 2, 2015
@earldouglas earldouglas changed the title Add/refactor tests for offline mode Web request blocking Add tests for offline mode Web request blocking Jan 2, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+2.16%) when pulling 6df462f on earldouglas:offline-mode into 044a225 on wikimedia:master.

});
it('should not allow latest content retrieval from storage', function() {
this.timeout(20000);
return assert.fails(
Copy link
Member

Choose a reason for hiding this comment

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

Why are we using assert.fails here,
Why not just return preq.get(...).then(function(e){assert.deepEqual(e.status, 500)}) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because .then(function(e) would not run -- it would be shortcutted by the throwing of e, so we need to wrap the whole thing in a try/catch, which is what assert.fails helps us do.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thats really useful, thanks for the explanation.

@hxjuneja
Copy link
Member

hxjuneja commented Jan 3, 2015

You can turn these off if you want https://travis-ci.org/wikimedia/restbase/jobs/45710252#L328? (if they are not logging some useful info, they make test look uglier ;) )

@earldouglas
Copy link
Contributor Author

@HardikJ How do I change the logging level (i.e. to hide info/request messages during testing)?

@hxjuneja
Copy link
Member

hxjuneja commented Jan 5, 2015

one way would be by changing the logging level to 'fatal' when running tests in offline mode

@earldouglas
Copy link
Contributor Author

Where in the code can I do this? Is it part of the opts.log object?

@hxjuneja
Copy link
Member

hxjuneja commented Jan 5, 2015

@earldouglas
Copy link
Contributor Author

Ah, of course. Thanks!

@coveralls
Copy link

Coverage Status

Coverage increased (+4.49%) when pulling a36d4c4 on earldouglas:offline-mode into 044a225 on wikimedia:master.

hxjuneja pushed a commit that referenced this pull request Jan 5, 2015
Add tests for offline mode Web request blocking
@hxjuneja hxjuneja merged commit 5c81803 into wikimedia:master Jan 5, 2015
@earldouglas earldouglas deleted the offline-mode branch January 6, 2015 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants