1193323 - remove eq_ and ok_: base

Status: RESOLVED FIXED

(Input Graveyard :: Code Quality, defect)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

We recently switched from nose to py.test. nose had helper functions eq_ and ok_ which we used extensively throughout the test suite. py.test doesn't need these since it does magic things with assert.

This bug covers removing eq_ and ok_ usage and replacing them with normal asserts in the fjord/base/ directory.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

Required skills:

* experience with Python
* ability to run the test suite

How to implement:

For each file in fjord/*/tests/, do the following:

1. remove eq_ and ok_ from the import list
2. go through the file converting eq_ and ok_ to appropriate assert lines

An eq_ example:

    eq_(resp.status_code, 200)

This line verifies that the value of resp.status_code is 200. The equivalent assert statement is:

    assert resp.status_code == 200

An ok_ example:

    ok_(user.is_admin())

This line verifies that the value returned by the call user.is_admin() is True. The equivalent assert statement is:

    assert user.is_admin()

You can read up on assert in the py.test docs:

    http://pytest.org/latest/assert.html

I did the work for fjord/alerts/. You can see the commit here:

    https://github.com/mozilla/fjord/commit/37cb87734570f15818061ac291f750931c896842


Marking this as a mentored bug. If you're interested in working on it,
please read through our "Join this project" guide [1], complete the
list of things you need to do before contributing and then add a comment
to this bug asking to have it assigned to you.

If you have any questions, please ask on the mailing list or in the
#input channel on irc.mozilla.org.

[1] http://fjord.readthedocs.org/en/latest/welcome.html

Comment 2

[reachtotj]

Hi,
I have successfully setup the development environment. eq_ and ok_ are defined in file "__init__.py". I am ready to work on this bug. From my understanding all I have to do is replace eq_ and ok_ with proper assert statements. like 
 eq_(body['redirect'], '/') will be
 assert body['redirect'] == '/'

Also if there is any message, then that will be added like
  assert body['redirect'] == '/', msg

If this bug is assigned and properly fixed, then I can also work on 1193321,1193324,1193325,1193326,1193327,1193328,1193330,1193331,1193332

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

This sounds perfect! I don't think any of the messages are particularly interesting. I think most of the time I included a message it was to make it easier to know which test data failed in cases where we have a bunch of test data being tested in a big for loop.

Otherwise this sounds great.

Assigning it to you now. Let me know if you have any problems.

Comment 4

[reachtotj]

When I replace the code with assert, to check I will simply have to run 

./py.test

Right?

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

That's right! To run the test suite, you run that specific command line. That runs the py.test script which sets up some environment stuff and then executes the py.test system to run all the tests.

The tests should all pass before and after your changes.

Comment 6

[Will Kahn-Greene [:willkg] ET needinfo? me]

In a PR: https://github.com/mozilla/fjord/pull/644

Landed in master: https://github.com/mozilla/fjord/commit/c4fab5c91086856cf58c5f09c0f1b3a457c2a572

This is tests-specific and doesn't need to be deployed, so marking this as FIXED.

Thank you!