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

Remove lingering nose functions from tests (post django 1.11 update) #7148

Merged

Conversation

Projects
None yet
3 participants
@jpetto
Copy link
Member

commented May 2, 2019

Description

A few nose functions remain after the Django 1.11 update.

@jpetto jpetto requested a review from jgmize May 2, 2019

@jgmize

jgmize approved these changes May 2, 2019

Copy link
Member

left a comment

one minor style nit, but I'm fine with merging as-is once tests pass

ok_('show_newsletter' in context)
eq_(True, context['show_newsletter'])
assert 'show_newsletter' in context
assert True == context['show_newsletter']

This comment has been minimized.

Copy link
@jgmize

jgmize May 2, 2019

Member

NIT: True == is redundant here

assert context['show_newsletter']

This comment has been minimized.

Copy link
@jgmize

jgmize May 2, 2019

Member

actually, the previous assertion is redundant too

ok_('show_newsletter' in context)
eq_(False, context['show_newsletter'])
assert 'show_newsletter' in context
assert False == context['show_newsletter']

This comment has been minimized.

Copy link
@pmac

pmac May 2, 2019

Member
Suggested change
assert False == context['show_newsletter']
assert not context['show_newsletter']
@jgmize

This comment has been minimized.

Copy link
Member

commented May 2, 2019

flake8 cares more about the style nits than I do:

bedrock/firefox/tests/test_base.py:388:21: E712 comparison to True should be 'if cond is True:' or 'if cond:'
bedrock/firefox/tests/test_base.py:399:22: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
2       E712 comparison to True should be 'if cond is True:' or 'if cond:'

@jpetto jpetto force-pushed the jpetto:remove-lingering-nose-functions-from-tests branch from 408cbf6 to e448c9e May 2, 2019

@jpetto

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Cleaned it up!

@jgmize jgmize merged commit fc3a4ec into mozilla:master May 2, 2019

2 checks passed

ci/circleci: test_js Your tests passed on CircleCI!
Details
ci/circleci: test_py Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.