#328 Fix tests and relative issues to tests
Merged 5 months ago by jkaluza. Opened 5 months ago by cqi.
cqi/freshmaker fix-tests  into  master

@@ -232,7 +232,7 @@ 

  

      freshmaker_images = []

      for image in security_images:

-         if re.match('.*\d{10}$', image['brew']['build']):

+         if re.match(r'.*\d{10}$', image['brew']['build']):

              freshmaker_images.append(image)

  

      logging.debug("All shipped containers with security fixes: ")

file modified
+2 -1

@@ -31,9 +31,10 @@ 

  

  from flask import g

  from flask_login import login_required as _login_required

+ from werkzeug.exceptions import Unauthorized

  

  from freshmaker import conf, log

- from freshmaker.errors import Unauthorized, Forbidden

+ from freshmaker.errors import Forbidden

  from freshmaker.models import User, commit_on_success

  

  

file modified
+1 -1

@@ -62,7 +62,7 @@ 

          Creates new ErrataAdvisory instance from the Erratum ID.

          """

          data = errata._get_advisory(errata_id)

-         erratum_data = data["errata"].values()

+         erratum_data = list(data["errata"].values())

          if not erratum_data:

              return None

          erratum_data = erratum_data[0]

file modified
+2 -5

@@ -24,6 +24,7 @@ 

  

  from flask import jsonify

  from freshmaker import app, log

+ from werkzeug.exceptions import Unauthorized

  

  

  class ValidationError(ValueError):

@@ -46,10 +47,6 @@ 

      pass

  

  

- class Unauthorized(ValueError):

-     pass

- 

- 

  class Forbidden(ValueError):

      pass

  

@@ -72,7 +69,7 @@ 

  @app.errorhandler(Unauthorized)

  def unauthorized_error(e):

      """Flask error handler for Unauthorized exceptions"""

-     return json_error(401, 'Unauthorized', e.args[0])

+     return json_error(401, 'Unauthorized', e.description)

  

  

  @app.errorhandler(Forbidden)

file modified
+1 -1

@@ -1150,7 +1150,7 @@ 

              images, get_nvr=lambda image: image['brew']['build'], reverse=True)

          images = []

          for k, v in groupby(sorted_images, key=lambda x: x['brew']['build']):

-             images.append(v.next())

+             images.append(six.next(v))

  

          # In case we query for unpublished images, we need to return just

          # the latest NVR for given name-version, otherwise images would

file modified
+1 -1

@@ -108,7 +108,7 @@ 

          work_queue_put(wrapped_msg)

      except ValueError as e:

          log.warn("No FreshmakerConsumer found.  Shutting down?  %r" % e)

-     except AttributeError as e:

+     except AttributeError:

          # In the event that `moksha.hub._hub` hasn't yet been initialized, we

          # need to store messages on the side until it becomes available.

          # As a last-ditch effort, try to hang initial messages in the config.

file modified
+4 -3

@@ -363,9 +363,10 @@ 

              packages.add(parsed_nvr["name"])

  

          # ODCS client expects list and not set for packages/builds, so convert

-         # them to lists.

-         builds = list(builds)

-         packages = list(packages)

+         # them to lists. Sorting the lists to make them easy to look up, e.g.

+         # in logs, and easy to test.

+         builds = sorted(builds)

+         packages = sorted(packages)

  

          if not self.handler.dry_run:

              with krb_context():

file modified
-1

@@ -20,7 +20,6 @@ 

  requests

  enum34 ; python_version <= '2.7'

  odcs[client]

- krbcontext

  dogpile.cache

  pyldap

  koji

file modified
+1

@@ -3,5 +3,6 @@ 

  copr

  mock

  pytest

+ pytest-cov

  vcrpy

  tox

file modified
+4 -4

@@ -25,6 +25,7 @@ 

  import flask

  

  from mock import patch, Mock

+ from werkzeug.exceptions import Unauthorized

  

  import freshmaker.auth

  

@@ -32,7 +33,6 @@ 

  from freshmaker.auth import load_krb_user_from_request

  from freshmaker.auth import load_openidc_user

  from freshmaker.auth import query_ldap_groups

- from freshmaker.errors import Unauthorized

  from freshmaker import app, db

  from freshmaker.models import User

  from tests.helpers import ModelsTestCase, FreshmakerTestCase

@@ -83,8 +83,8 @@ 

          with app.test_request_context():

              with self.assertRaises(Unauthorized) as ctx:

                  load_krb_user_from_request(flask.request)

-             self.assertTrue(

-                 'REMOTE_USER is not present in request.' in ctx.exception.args)

+             self.assertIn('REMOTE_USER is not present in request.',

+                           ctx.exception.description)

  

  

  class TestLoadOpenIDCUserFromRequest(ModelsTestCase):

@@ -187,7 +187,7 @@ 

                      load_openidc_user(flask.request)

                  self.assertTrue(

                      'Required OIDC scope new-compose not present.' in

-                     ctx.exception.args)

+                     ctx.exception.description)

  

  

  class TestQueryLdapGroups(FreshmakerTestCase):

file modified
+6 -6

@@ -364,7 +364,7 @@ 

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'MyHandler': {

              'image': {

-                 'advisory_name': 'RHSA-\d+:\d+'

+                 'advisory_name': r'RHSA-\d+:\d+'

              }

          }

      })

@@ -381,7 +381,7 @@ 

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'MyHandler': {

              'image': {

-                 'advisory_name': 'RHSA-\d+:\d+',

+                 'advisory_name': r'RHSA-\d+:\d+',

                  'advisory_state': 'REL_PREP'

              }

          }

@@ -401,7 +401,7 @@ 

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'MyHandler': {

              'image': any_(

-                 {'advisory_name': 'RHSA-\d+:\d+'},

+                 {'advisory_name': r'RHSA-\d+:\d+'},

                  {'advisory_state': 'REL_PREP'},

              )

          }

@@ -421,7 +421,7 @@ 

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'MyHandler': {

              'image': all_(

-                 {'advisory_name': 'RHSA-\d+:\d+'},

+                 {'advisory_name': r'RHSA-\d+:\d+'},

                  any_(

                      {'has_hightouch_bugs': True},

                      {'severity': ['critical', 'important']}

@@ -457,12 +457,12 @@ 

  

      @patch.object(freshmaker.conf, 'handler_build_whitelist', new={

          'MyHandler': {

-             'image': {'advisory_name': 'RHSA-\d+:\d+'},

+             'image': {'advisory_name': r'RHSA-\d+:\d+'},

          }

      })

      @patch.object(freshmaker.conf, 'handler_build_blacklist', new={

          'MyHandler': {

-             'image': {'advisory_name': 'RHSA-2016:\d+'},

+             'image': {'advisory_name': r'RHSA-2016:\d+'},

          }

      })

      def test_blacklist(self):

file modified
+13 -4

@@ -899,17 +899,17 @@ 

                              {

                                  "field": "repositories.*.tags.*.name",

                                  "op": "=",

-                                 "rvalue": "tag1"

+                                 "rvalue": "latest"

                              },

                              {

                                  "field": "repositories.*.tags.*.name",

                                  "op": "=",

-                                 "rvalue": "tag2"

+                                 "rvalue": "tag1"

                              },

                              {

                                  "field": "repositories.*.tags.*.name",

                                  "op": "=",

-                                 "rvalue": "latest"

+                                 "rvalue": "tag2"

                              },

                          ],

                      },

@@ -937,7 +937,16 @@ 

              "projection": lb._get_default_projection(srpm_names=["openssl"])

          }

  

-         cont_images.assert_called_with(expected_image_request)

+         # auto_rebuild_tags is a set in the source code. When generate

+         # criteria for tags, the order is not guaranteed. Following lines sort

+         # the tags criteria in order to assert with expected value.

+         args, _ = cont_images.call_args

+         request_arg = args[0]

+         tags_criteira = request_arg['query']['$and'][1]['$or']

+         request_arg['query']['$and'][1]['$or'] = sorted(

+             tags_criteira, key=lambda item: item['rvalue'])

+ 

+         self.assertEqual(expected_image_request, request_arg)

  

          # Only the second image should be returned, because the first one

          # is in repository "product1/repo1", but we have asked for images

file modified
+1 -1

@@ -22,7 +22,7 @@ 

  import fedmsg.config

  import mock

  import freshmaker

- import queue

+ from six.moves import queue

  

  from freshmaker import app, db, events, models, login_manager

  from tests import helpers

file modified
+3 -3

@@ -24,7 +24,7 @@ 

  

  import fedmsg

  import six

- import queue

+ from six.moves import queue

  

  from mock import patch, Mock, MagicMock

  from odcs.client.odcs import AuthMech

@@ -299,8 +299,8 @@ 

  

          # Ensure new_compose is called to request a new compose

          odcs.new_compose.assert_called_once_with(

-             '', 'build', builds=['avalon-logkit-2.1-14.el7', 'apache-commons-lang-2.6-15.el7'],

-             flags=['no_deps'], packages=[u'avalon-logkit', u'apache-commons-lang'], sigkeys=[])

+             '', 'build', builds=['apache-commons-lang-2.6-15.el7', 'avalon-logkit-2.1-14.el7'],

+             flags=['no_deps'], packages=[u'apache-commons-lang', u'avalon-logkit'], sigkeys=[])

  

      def _create_consumer(self):

          hub = MagicMock()

file modified
+5 -4

@@ -22,10 +22,11 @@ 

  #

  # Written by Jan Kaluza <jkaluza@redhat.com>

  

- from mock import patch, MagicMock

  import koji

  import fedmsg.config

- import queue

+ 

+ from mock import patch, MagicMock

+ from six.moves import queue

  

  import freshmaker

  from freshmaker import db

@@ -99,7 +100,7 @@ 

          hub = MagicMock()

          producer = FreshmakerProducer(hub)

          producer.check_unfinished_koji_tasks(db.session)

-         self.assertRaises(consumer.incoming.get, block=False)

+         self.assertRaises(queue.Empty, consumer.incoming.get, block=False)

  

      @patch('freshmaker.kojiservice.KojiService.get_task_info')

      @patch("freshmaker.consumer.get_global_consumer")

@@ -113,4 +114,4 @@ 

          hub = MagicMock()

          producer = FreshmakerProducer(hub)

          producer.check_unfinished_koji_tasks(db.session)

-         self.assertRaises(consumer.incoming.get, block=False)

+         self.assertRaises(queue.Empty, consumer.incoming.get, block=False)

file modified
+15 -14

@@ -4,25 +4,14 @@ 

  # and then run "tox" from this directory.

  

  [tox]

- envlist = py27, py35, coverage, flake8, bandit

+ envlist = py27, py35, flake8, bandit

  

  [testenv]

- sitepackages=True

  skip_install = True

  deps = -r{toxinidir}/test-requirements.txt

  commands =

      py.test {posargs}

  

- [testenv:coverage]

- basepython = python3

- deps =

-     {[testenv]deps}

-     coverage

- commands =

-     coverage run --parallel-mode -m pytest

-     coverage combine

-     coverage report --omit=tests/*,.tox/*,/usr/* -m --skip-covered

- 

  [testenv:flake8]

  basepython = python3

  skip_install = true

@@ -38,5 +27,17 @@ 

  ignore_outcome = True

  

  [flake8]

- ignore = E501,E731

- exclude = freshmaker/migrations/*,.tox/*,build/*,__pycache__,scripts/print_handlers_md.py,.copr/*

+ ignore = E501,E731,W504

+ exclude = freshmaker/migrations/*,.tox/*,build/*,__pycache__,scripts/print_handlers_md.py,.copr/*,.env

+ 

+ [pytest]

+ addopts = --cov=freshmaker

+ 

+ [coverage:report]

+ skip_covered = 1

+ show_missing = 1

+ omit =

+      .tox

+      .env

+      tests/*

+      /usr/* 

\ No newline at end of file

no initial comment

Pretty please pagure-ci rebuild

:+1: Looks great! This fixes py35 and flake8 for me.

I don't think the Jenkins job is working properly for freshmaker. Last run was on Oct 12, 2018.

Also, looking at the Jenkinsfile, only flake8 runs. The unit tests are not executed. We should fix that too, but I'm assuming this is probably a bigger task. Let's do it on a different PR.

Unit tests for py27 are still broken on my env. I had to create a symlink to even get the tests started:

$ sudo ln -s $PWD/conf/configrh.py /etc/freshmaker/config.py

But then it fails due to prometheus_client import error: ImportError: No module named prometheus_client

This is probably because we're using system site packages.
If we stop using system site packages, it fails in py27 with ImportError for queue module this time. This is because the module is Queue in python 2. So... let's use six.moves to fix this particular problem. This patch on top of this PR did it for me:

diff --git a/tests/test_monitor.py b/tests/test_monitor.py
index 046c108..5ed025d 100644
--- a/tests/test_monitor.py
+++ b/tests/test_monitor.py
@@ -22,9 +22,9 @@
 import fedmsg.config
 import mock
 import freshmaker
-import queue

 from freshmaker import app, db, events, models, login_manager
+from six.moves import queue
 from tests import helpers


diff --git a/tests/test_odcsclient.py b/tests/test_odcsclient.py
index eb89982..49fdaf3 100644
--- a/tests/test_odcsclient.py
+++ b/tests/test_odcsclient.py
@@ -24,10 +24,10 @@

 import fedmsg
 import six
-import queue

 from mock import patch, Mock, MagicMock
 from odcs.client.odcs import AuthMech
+from six.moves import queue

 from freshmaker import conf, db
 from freshmaker.models import Event, ArtifactBuild, Compose
diff --git a/tests/test_producer.py b/tests/test_producer.py
index 4e2d1f7..af462bc 100644
--- a/tests/test_producer.py
+++ b/tests/test_producer.py
@@ -23,9 +23,9 @@
 # Written by Jan Kaluza <jkaluza@redhat.com>

 from mock import patch, MagicMock
+from six.moves import queue
 import koji
 import fedmsg.config
-import queue

 import freshmaker
 from freshmaker import db
diff --git a/tox.ini b/tox.ini
index 9c83f0e..9943d8a 100644
--- a/tox.ini
+++ b/tox.ini
@@ -7,7 +7,6 @@
 envlist = py27, py35, coverage, flake8, bandit

 [testenv]
-sitepackages=True
 skip_install = True
 deps = -r{toxinidir}/test-requirements.txt
 commands =

3 new commits added

  • Remove krbcontext from requirements.txt
  • Adjust tox to run coverage
  • sitepackages is not necessary to run tests
5 months ago

@lucarval

Unit tests for py27 are still broken on my env. I had to create a symlink to even get the tests started:
$ sudo ln -s $PWD/conf/configrh.py /etc/freshmaker/config.py

I don't see this issue in my system. My steps to run tox are:

python3 -m venv .env
. .env/bin/activate
pip install -r test-requirements.txt
tox

New commits are added. sitepackages is not necessary to run tests and is disabled. I also refactored testenv for coverage.

I think @lucarval did not run the tox, but nosetests maybe? Not sure though.

Tests are passing for me locally, merging.

Commit be5689e fixes this pull-request

Pull-Request has been merged by jkaluza

5 months ago

Pull-Request has been merged by jkaluza

5 months ago