1193331 - remove eq_ and ok_: suggest

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/suggest/ 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

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

Assigning these all to reachtotj since he's cruised through the last couple.

Comment 3

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

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

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

This only affects tests and nothing needs to be deployed. Marking as FIXED.

Thank you!

Comment 4

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

Oops--I shouldn't have closed this. There are still eq_ and ok_ uses in fjord/suggest/providers/sumo/tests/test_sumosuggest.py .

Can you do those, too?

Comment 5

[reachtotj]

Ohh I was looking under main tests directory. I will fix this and take care from the next time

Comment 6

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

sumosuggest provider tests in PR: https://github.com/mozilla/fjord/pull/654

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

I double-checked the fjord/suggest/ directory tree for eq_ and ok_ and we're good to go.

Since this is specific to the tests and doesn't affect servers, I'm going to mark this FIXED.

Thank you, again!