Add tests for OPTIONS method in users and teams modules #34

khorolets · 2016-09-30T11:56:32Z

No description provided.

frol · 2016-09-30T11:59:10Z

tests/modules/teams/conftest.py

@@ -24,3 +24,20 @@ def team_for_regular_user(db, regular_user, readonly_user):
    db.session.delete(regular_user_team_member)
    db.session.delete(team)
    db.session.commit()
+
+@pytest.yield_fixture()
+def team_for_nobody(db):  # pylint: disable=invalid-name


I suggest we create another tests/.pylincrc, where we disable these and some other checks.

Where do you use this fixture? O_o

My bad, I've renamed it and then forget to use new name.

About pylint, what do you want me to do? Should I create rcfile or remove this disable?

Try to copy the existing .pylintrc to tests, and disable all unnecessary checks (all that are currently complained about).

I also wonder if it supports nesting, e.g. if we just put a tests/.pylintrc with a few lines of config instead of a complete copy...

frol · 2016-09-30T12:11:39Z

tests/modules/teams/resources/test_options.py

+    ('/api/v1/teams/', 401),
+    ('/api/v1/users/1', 401),
+))
+def test_users_options_unauthorized(path, status_code, flask_app_client):


I think it is worth to rename these functions. users in not an appropriate name in teams module.

coveralls · 2016-09-30T12:29:57Z

Coverage remained the same at 91.386% when pulling c9035e6 on khorolets:options_tests into de01c7a on frol:master.

coveralls · 2016-09-30T12:34:34Z

Coverage remained the same at 91.386% when pulling 7ca37ae on khorolets:options_tests into de01c7a on frol:master.

frol · 2016-09-30T12:47:59Z

tests/modules/users/resources/test_options.py

+    assert response.status_code == status_code
+    if result:
+        assert len(
+                list(set(result) - set(response.headers['Allow'].replace(' ', '').split(',')))


Just for your info, there is no point in converting set to list since len() works just fine on sets.

Also, there is no point in doing len(...) == 0, it is just less pythonic, though I think you know. You may also learn about any and all.

Also, split() can handle a string as a parameter, which it will handle as a list of possible separators (not like one single multicharacter separator), so

>>> 'A, B'.split(', ') ['A', 'B']

Ultimately, I would write this check like this:

assert not (set(result) - set(response.headers['Allow'].split(', ')))

frol · 2016-09-30T12:53:09Z

tests/modules/teams/resources/test_options.py

+        response = flask_app_client.options(path)
+
+    assert response.status_code == 204
+    assert not (set(result) - set(response.headers['Allow'].split(', ')))


@codacy-bot Are you mad? O_o

@frol They are really unnecessary, no?

Oh, actually, the better way is to compare sets because you don't want the test pass when the expected result is ['OPTIONS'], and returned value is ['OPTIONS', 'GET']. So:

assert set(result) == set(response.headers['Allow'].split(', '))

I would also rename result into expected_allowed_methods, and define them as sets from the beginning.

coveralls · 2016-09-30T12:55:36Z

Coverage remained the same at 91.386% when pulling fa80b14 on khorolets:options_tests into de01c7a on frol:master.

frol · 2016-09-30T12:57:58Z

tests/modules/teams/resources/test_options.py

+        response = flask_app_client.options(path)
+
+    assert response.status_code == 204
+    assert not (set(result) - set(response.headers['Allow'].split(', ')))


Oh, actually, the better way is to compare sets because you don't want the test pass when the expected result is ['OPTIONS'], and returned value is ['OPTIONS', 'GET']. So:

assert set(result) == set(response.headers['Allow'].split(', '))

I would also rename result into expected_allowed_methods, and define them as sets from the beginning.

coveralls · 2016-09-30T12:58:04Z

Coverage remained the same at 91.386% when pulling 966a8b9 on khorolets:options_tests into de01c7a on frol:master.

coveralls · 2016-09-30T13:08:13Z

Coverage remained the same at 91.386% when pulling 539e45a on khorolets:options_tests into de01c7a on frol:master.

frol · 2016-09-30T13:15:45Z

tests/modules/teams/resources/test_options.py

+        response = flask_app_client.options(path)
+
+    assert response.status_code == 204
+    assert expected_allowed_methods == set(response.headers['Allow'].split(', '))


I will drive you crazy, but I want you change this thing once (I hope last) time again. Just like with the line above, you write and expression on the left, and "const" on the right... So I would like to swap the expected_allowed_methods and the right side.

Ha! I was teaching at school for almost 3 years. You won't drive me crazy so easy :)

coveralls · 2016-09-30T13:38:06Z

Coverage remained the same at 91.386% when pulling bc3b520 on khorolets:options_tests into de01c7a on frol:master.

frol requested changes Sep 30, 2016

View reviewed changes

khorolets force-pushed the options_tests branch 2 times, most recently from c9035e6 to 7ca37ae Compare September 30, 2016 12:29

frol requested changes Sep 30, 2016

View reviewed changes

khorolets force-pushed the options_tests branch from 7ca37ae to fa80b14 Compare September 30, 2016 12:50

frol reviewed Sep 30, 2016

View reviewed changes

khorolets force-pushed the options_tests branch from fa80b14 to 966a8b9 Compare September 30, 2016 12:53

frol requested changes Sep 30, 2016

View reviewed changes

khorolets force-pushed the options_tests branch from 966a8b9 to 539e45a Compare September 30, 2016 13:03

frol requested changes Sep 30, 2016

View reviewed changes

Add tests for OPTIONS method in users and teams modules

bc3b520

khorolets force-pushed the options_tests branch from 539e45a to bc3b520 Compare September 30, 2016 13:33

frol approved these changes Sep 30, 2016

View reviewed changes

frol merged commit 64bc2ad into frol:master Sep 30, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for OPTIONS method in users and teams modules #34

Add tests for OPTIONS method in users and teams modules #34

khorolets commented Sep 30, 2016

frol Sep 30, 2016

frol Sep 30, 2016

khorolets Sep 30, 2016

khorolets Sep 30, 2016

frol Sep 30, 2016

frol Sep 30, 2016

frol Sep 30, 2016

coveralls commented Sep 30, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited

frol Sep 30, 2016

frol Sep 30, 2016

khorolets Sep 30, 2016

frol Sep 30, 2016

coveralls commented Sep 30, 2016 •

edited

frol Sep 30, 2016

coveralls commented Sep 30, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited

frol Sep 30, 2016

khorolets Sep 30, 2016

coveralls commented Sep 30, 2016 •

edited

Add tests for OPTIONS method in users and teams modules #34

Add tests for OPTIONS method in users and teams modules #34

Conversation

khorolets commented Sep 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 30, 2016 • edited

coveralls commented Sep 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 30, 2016 • edited

Choose a reason for hiding this comment

coveralls commented Sep 30, 2016 • edited

coveralls commented Sep 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 30, 2016 • edited

coveralls commented Sep 30, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited

coveralls commented Sep 30, 2016 •

edited