test: Start integrating pytest #6827

me-diru · 2020-02-09T04:20:14Z

Reference #6503

Short description of what this resolves:

Converts test code to pytest standard

Changes proposed in this pull request:

Change in test code

Checklist

I have read the Contribution & Best practices Guide and my PR follows them.
My branch is up-to-date with the Upstream development branch.
The unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

All the functions created/modified in this PR contain relevant docstrings.

houndci-bot · 2020-02-09T04:20:47Z

tests/all/unit/api/helpers/test_errors.py

+    """To test details in the form of dict"""
+    error_response = ErrorResponse(source="test source", detail="test detail")
+    expected_dict = {
+        'status': error_response.status,


Black would make changes.

Pre-commit ran black

codecov · 2020-02-09T04:25:42Z

Codecov Report

Merging #6827 into development will decrease coverage by 6.07%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           development    #6827      +/-   ##
===============================================
- Coverage        66.94%   60.87%   -6.08%     
===============================================
  Files              314      259      -55     
  Lines            15423    13564    -1859     
===============================================
- Hits             10325     8257    -2068     
- Misses            5098     5307     +209

Impacted Files	Coverage Δ
app/api/helpers/scheduled_jobs.py	`48.29% <ø> (ø)`
app/models/event.py	`81.27% <ø> (ø)`
populate_db.py	`90.52% <ø> (ø)`
app/factories/microlocation.py
tests/all/integration/auth_helper.py
tests/all/integration/api/helpers/test_files.py
...ntegration/api/helpers/test_systemnotifications.py
...all/integration/api/helpers/test_scheduled_jobs.py
app/factories/export_job.py
tests/all/unit/graphql/utils/test_fields.py
... and 64 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41adf3e...f1abace. Read the comment docs.

mrsaicharan1

Looks fine to me

me-diru · 2020-02-09T06:03:48Z

@iamareebjamal Can you please take a look at this?

iamareebjamal · 2020-02-09T08:01:13Z

This will close the issue when it just changes 1 test. Besides, the main tests we want to change are db tests

me-diru · 2020-02-09T08:05:12Z

This will close the issue when it just changes 1 test. Besides, the main tests we want to change are db tests

Yes, I just wanted to know if I was going in the right direction on this. I am planning to change all the tests in a similar way in this PR itself.

iamareebjamal · 2020-02-09T08:10:52Z

Just change 1 DB test for now

houndci-bot · 2020-02-11T11:17:37Z

tests/all/integration/api/helpers/test_auth.py


            user = create_user(email='[email protected]', password='password')
            user.is_admin = False
            status = AuthManager.check_auth_admin('[email protected]', 'password')
-            self.assertEqual(False, status)
+            assert False == status


comparison to False should be 'if cond is False:' or 'if not cond:'

doesn't apply here.

houndci-bot · 2020-02-11T11:17:37Z

tests/all/integration/api/helpers/test_auth.py

@@ -42,12 +42,12 @@ def test_check_auth_admin(self):
            user = create_user(email='[email protected]', password='password')
            user.is_admin = True
            status = AuthManager.check_auth_admin('[email protected]', 'password')
-            self.assertEqual(True, status)
+            assert True == status


comparison to True should be 'if cond is True:' or 'if cond:'

houndci-bot · 2020-02-11T11:17:38Z

tests/all/integration/api/helpers/test_auth.py

@@ -33,7 +33,7 @@ def test_is_accessible(self):
            user = create_user(email='[email protected]', password='password')
            login_user(user)
            logout_user()
-            self.assertEqual(AuthManager.is_accessible(), False)
+            assert AuthManager.is_accessible() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2020-02-11T11:17:38Z

tests/all/integration/api/helpers/test_auth.py

@@ -24,7 +24,7 @@ def test_verified_user(self):
            user = create_user(email='[email protected]', password='password')
            user.is_verified = False
            login_user(user)
-            self.assertEqual(AuthManager.is_verified_user(), False)
+            assert AuthManager.is_verified_user() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

iamareebjamal · 2020-02-11T11:55:23Z

tests/all/integration/api/helpers/test_auth.py

@@ -15,7 +15,7 @@ def test_load_user(self):

        with self.app.test_request_context():
            user = create_user(email='[email protected]', password='password')
-            self.assertEqual(user, db.session.query(User).get(user.id))
+            assert user == db.session.query(User).get(user.id)


Nope. It is still not pytest test. This is standard unit test

houndci-bot · 2020-02-13T03:34:48Z

tests/all/integration/api/helpers/test_auth.py

+        user = create_user(email='[email protected]', password='password')
+        user.is_admin = False
+        status = AuthManager.check_auth_admin('[email protected]', 'password')
+        assert False == status


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2020-02-13T03:34:48Z

tests/all/integration/api/helpers/test_auth.py

+        user = create_user(email='[email protected]', password='password')
+        user.is_admin = True
+        status = AuthManager.check_auth_admin('[email protected]', 'password')
+        assert True == status


comparison to True should be 'if cond is True:' or 'if cond:'

houndci-bot · 2020-02-13T03:34:48Z

tests/all/integration/api/helpers/test_auth.py

+        user = create_user(email='[email protected]', password='password')
+        login_user(user)
+        logout_user()
+        assert AuthManager.is_accessible() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2020-02-13T03:34:48Z

tests/all/integration/api/helpers/test_auth.py

+        user = create_user(email='[email protected]', password='password')
+        user.is_verified = False
+        login_user(user)
+        assert AuthManager.is_verified_user() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2020-02-13T03:34:49Z

tests/all/integration/api/helpers/test_auth.py

+from tests.all.integration.setup_database import Setup
+
+
+@pytest.fixture(scope='module')


Black would make changes.

houndci-bot · 2020-02-13T03:34:49Z

tests/all/integration/api/helpers/test_auth.py

 from flask_login import login_user, logout_user
-
+from flask import Flask, session


'flask.Flask' imported but unused
'flask.session' imported but unused
redefinition of unused 'Flask' from line 2

me-diru · 2020-02-13T08:17:01Z

@iamareebjamal @mrsaicharan1 can you please take a look at this?

iamareebjamal · 2020-02-13T08:41:20Z

Yes, I did. And the build is failing

me-diru · 2020-02-13T09:53:20Z

The tests are passing locally when I run using pytest

me-diru · 2020-02-14T11:54:08Z

@iamareebjamal Can you please review this?
I think it's failing because travis build runs nosetest command.

iamareebjamal · 2020-02-14T12:04:45Z

Then change it to correct command

me-diru · 2020-02-16T16:29:23Z

do I have to add pytest in requirements and change travis yml file ?

iamareebjamal · 2020-02-16T16:50:03Z

You may

iamareebjamal · 2020-02-16T19:49:40Z

.travis.yml

-  - nosetests tests/ -v --with-coverage
+script: |
+  nosetests tests/ -v --with-coverage -I tests/all/integration/api/helpers/test_auth.py
+  pytest tests/all/integration/api/helpers/test_auth.py


I didn't know another way to do it.
Somehow I have to exclude test_auth from nostests and test it with pytest

iamareebjamal · 2020-02-16T19:49:47Z

.travis.yml

@@ -40,3 +41,4 @@ branches:
  only:
    - master
    - development
+    - pytest


iamareebjamal · 2020-02-16T20:25:03Z

.travis.yml

-script:
-  - nosetests tests/ -v --with-coverage
+script: 
+  - nosetests tests/ --ignore-files="test_auth\.py" -v --with-coverage 


sir, nosetests fails for a pytest code. Should I add a pytest script in yml file?

All tests should run with 1 script

houndci-bot · 2020-05-06T22:27:26Z

tests/all/integration/api/helpers/test_auth.py

+    user = create_user(email='[email protected]', password='password')
+    user.is_admin = False
+    status = AuthManager.check_auth_admin('[email protected]', 'password')
+    assert False == status


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2020-05-06T22:27:26Z

tests/all/integration/api/helpers/test_auth.py

+    user = create_user(email='[email protected]', password='password')
+    user.is_admin = True
+    status = AuthManager.check_auth_admin('[email protected]', 'password')
+    assert True == status


comparison to True should be 'if cond is True:' or 'if cond:'

houndci-bot · 2020-05-06T22:27:27Z

tests/all/integration/api/helpers/test_auth.py

+    user = create_user(email='[email protected]', password='password')
+    login_user(user)
+    logout_user()
+    assert AuthManager.is_accessible() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2020-05-06T22:27:27Z

tests/all/integration/api/helpers/test_auth.py

+    user = create_user(email='[email protected]', password='password')
+    user.is_verified = False
+    login_user(user)
+    assert AuthManager.is_verified_user() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

niranjan94 · 2020-05-07T05:15:46Z

Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- tests/all/integration/api/validation/test_created_at.py  5
- tests/all/unit/api/helpers/test_errors.py  5
- tests/all/integration/api/helpers/test_auth.py  1

See the complete overview on Codacy

changed test_errors to pytest

76a5942

auto-label bot added the testing label Feb 9, 2020

me-diru requested a review from mrsaicharan1 February 9, 2020 04:20

houndci-bot reviewed Feb 9, 2020

View reviewed changes

mrsaicharan1 previously approved these changes Feb 9, 2020

View reviewed changes

modified test_auth to pytest

35d50fa

me-diru dismissed mrsaicharan1’s stale review via 35d50fa February 11, 2020 11:17

houndci-bot reviewed Feb 11, 2020

View reviewed changes

iamareebjamal reviewed Feb 11, 2020

View reviewed changes

added pytest fixture to test_auth

a9bae16

houndci-bot reviewed Feb 13, 2020

View reviewed changes

removed hound errors

c7b8b65

build fail removed in test_auth.py

43eb8c2

removed unused import

2b2ebb3

changed travis code and added pytest

80ebfad

iamareebjamal reviewed Feb 16, 2020

View reviewed changes

removed unused commands

0210840

changed test command in travis

b51136c

iamareebjamal reviewed Feb 16, 2020

View reviewed changes

me-diru and others added 4 commits February 20, 2020 20:50

changed to single command

b8fda86

Merge branch 'development' into pytest

e9ac3d7

finalize

55f5f45

finalize

ff0b7a7

houndci-bot reviewed May 6, 2020

View reviewed changes

iamareebjamal added 3 commits May 7, 2020 04:07

finalize

fa5d264

finalize

c70976b

omit

f1abace

iamareebjamal changed the title ~~test : Changed test_erros to pytest~~ test : Start integrating pytest May 7, 2020

iamareebjamal approved these changes May 7, 2020

View reviewed changes

iamareebjamal changed the title ~~test : Start integrating pytest~~ test: Start integrating pytest May 7, 2020

iamareebjamal merged commit 82aab77 into fossasia:development May 7, 2020

		from tests.all.integration.setup_database import Setup


		@pytest.fixture(scope='module')

		from flask_login import login_user, logout_user

		from flask import Flask, session

test: Start integrating pytest #6827

test: Start integrating pytest #6827

Conversation

me-diru commented Feb 9, 2020 • edited by iamareebjamal Loading

Short description of what this resolves:

Changes proposed in this pull request:

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 9, 2020 • edited Loading

Codecov Report

mrsaicharan1 left a comment

Choose a reason for hiding this comment

me-diru commented Feb 9, 2020

iamareebjamal commented Feb 9, 2020

me-diru commented Feb 9, 2020 • edited Loading

iamareebjamal commented Feb 9, 2020

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

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

me-diru commented Feb 13, 2020

iamareebjamal commented Feb 13, 2020

me-diru commented Feb 13, 2020 • edited Loading

me-diru commented Feb 14, 2020

iamareebjamal commented Feb 14, 2020

me-diru commented Feb 16, 2020

iamareebjamal commented Feb 16, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niranjan94 commented May 7, 2020

me-diru commented Feb 9, 2020 •

edited by iamareebjamal

Loading

codecov bot commented Feb 9, 2020 •

edited

Loading

me-diru commented Feb 9, 2020 •

edited

Loading

me-diru commented Feb 13, 2020 •

edited

Loading