#396 Add a --scenario argument to CLI
Merged 3 years ago by vmaljulin. Opened 3 years ago by vmaljulin.
vmaljulin/waiverdb RHELWF-1882  into  master

file modified
+11
@@ -43,6 +43,10 @@ 

  

      Specify a testcase for the subject.

  

+ .. option:: -S, --scenario SCENARIO

+ 

+     Specify a scenario for a result to waive.

+ 

  .. option:: -p, --product-version TEXT

  

      Specify one of PDC's product version identifiers.
@@ -86,3 +90,10 @@ 

      waiverdb-cli -t dist.rpmdeplint \

          -s '{"item": "qclib-1.3.1-3.fc28", "type": "koji_build"}' \

          -p "fedora-28" -c "This is expected for non-x86 packages"

+ 

+ Waive test results with a specific subject and scenario::

+ 

+     waiverdb-cli -t update.install_default_update_live \

+         -i FEDORA-2020-a70501de3d -T koji_build \

+         -S "fedora.updates-everything-boot-iso.x86_64.uefi" \

+         -c "This is ok"

file modified
+2
@@ -9,6 +9,8 @@ 

  SQLAlchemy

  gssapi

  flask-oidc

+ Flask-Migrate

+ stomp.py

  

  # Documentation requirements

  sphinx

file modified
+50
@@ -2,6 +2,7 @@ 

  import pytest

  import json

  from mock import Mock, patch

+ from textwrap import dedent

  from click.testing import CliRunner

  from waiverdb import __version__

  from waiverdb.cli import cli as waiverdb_cli
@@ -313,6 +314,19 @@ 

      assert result.output == 'Error: Please specify result_id or subject/testcase. Not both\n'

  

  

+ def test_malformed_submission_with_id_and_scenario(tmpdir):

+     runner = CliRunner()

+     p = tmpdir.join('client.conf')

+     p.write(dedent("""

+         [waiverdb]

+         auth_method=dummy

+         api_url=http://localhost:5004/api/v1.0

+     """))

+     args = ['-C', p.strpath, '-p', 'Parrot', '-r', '123', '-S', 'somescenario', '-c', "This is OK"]

+     result = runner.invoke(waiverdb_cli, args, catch_exceptions=False)

+     assert result.output == 'Error: Please specify result_id or scenario. Not both\n'

+ 

+ 

  def test_submit_waiver_for_original_spec_nvr_result(tmpdir):

      with patch('requests.request') as mock_request:

          mock_rv = Mock()
@@ -389,6 +403,42 @@ 

          )

  

  

+ def test_create_waiver_product_version_from_koji_build_scenario(tmpdir):

+     with patch('requests.request') as mock_request:

+         mock_rv = Mock()

+         mock_rv.json.return_value = [{

+             "comment": "This is fine",

+             "id": 15,

+             "subject_type": "koji_build",

+             "subject_identifier": "setup-2.8.71-7.el7_4",

+             "scenario": "somescenario",

+             "testcase": "test.testcase",

+             "timestamp": "2017-010-16T17:42:04.209638",

+             "username": "foo",

+             "waived": True

+         }]

+         mock_request.return_value = mock_rv

+         p = tmpdir.join('client.conf')

+         p.write(dedent("""

+             [waiverdb]

+             auth_method=dummy

+             api_url=http://localhost:5004/api/v1.0

+             koji_base_url=https://koji.fedoraproject.org/kojihub

+         """))

+         runner = CliRunner()

+         args = [

+             '-C', p.strpath, '-s', '{"type": "koji_build", "item": "setup-2.8.71-7.el7_4"}',

+             '-S', 'somescenario', '-t', 'test.testcase', '-c', "This is fine"

+         ]

+         result = runner.invoke(waiverdb_cli, args, catch_exceptions=False)

+         mock_request.assert_called()

+         assert result.output == (

+             'Created waiver 15 for result with '

+             'subject type koji_build, identifier setup-2.8.71-7.el7_4 '

+             'and testcase test.testcase, scenario is somescenario\n'

+         )

+ 

+ 

  def test_create_waiver_product_version_from_compose(tmpdir):

      with patch('requests.request') as mock_request:

          mock_rv = Mock()

file modified
+14 -3
@@ -86,6 +86,8 @@ 

              waiver_id = data['id']

              msg = 'subject type {0}, identifier {1} and testcase {2}'.format(

                  data['subject_type'], data['subject_identifier'], data['testcase'])

+             if data.get('scenario'):

+                 msg += f", scenario is {data['scenario']}"

              print_result(waiver_id, msg)

  

  
@@ -122,6 +124,8 @@ 

                help='Specify a config file to use')

  @click.option('--result-id', '-r', multiple=True, type=int,

                help='Specify one or more results to be waived')

+ @click.option('--scenario', '-S',

+               help='Specify a scenario for a result to waive')

  @click.option('--subject', '-s', type=OldJSONSubject(),

                help=('Deprecated. Use --subject-identifier and --subject-type instead. '

                      'Subject for a result to waive.'))
@@ -139,8 +143,8 @@ 

                help='A comment explaining why the result is waived')

  @click.option('--username', '-u', default=None,

                help='Username on whose behalf the caller is proxying.')

- def cli(username, comment, waived, product_version, testcase, subject, subject_identifier,

-         subject_type, result_id, config_file):

+ def cli(username, comment, waived, product_version, testcase, scenario, subject,

+         subject_identifier, subject_type, result_id, config_file):

      """

      Creates new waiver against test results.

  
@@ -150,8 +154,12 @@ 

          waiverdb-cli -r 47 -r 48 -p "fedora-28" -c "This is fine"

  

      \b

-         waiverdb-cli -t dist.rpmdeplint -i qclib-1.3.1-3.fc28 -T koji_build \\

+         waiverdb-cli -t dist.rpmdeplint -i qclib-1.3.1-3.fc28 -T bodhi_update \\

              -p "fedora-28" -c "This is expected for non-x86 packages"

+ 

+     \b

+         waiverdb-cli -t update.install_default_update_live -i FEDORA-2020-a70501de3d \\

+             -T koji_build -S "fedora.updates-everything-boot-iso.x86_64.uefi" -c "This is ok"

      """

      config = configparser.ConfigParser()

  
@@ -179,6 +187,8 @@ 

          raise click.ClickException('Please specify comment')

      if result_ids and (testcase or subject_identifier):

          raise click.ClickException('Please specify result_id or id/type/testcase. Not both')

+     if result_ids and scenario:

+         raise click.ClickException('Please specify result_id or scenario. Not both')

Add test for this.

      if not result_ids and not subject_identifier:

          raise click.ClickException('Please specify subject-identifier')

      if not result_ids and not testcase:
@@ -223,6 +233,7 @@ 

              'subject_type': subject_type,

              'testcase': testcase,

              'waived': waived,

+             'scenario': scenario,

              'product_version': product_version,

              'comment': comment,

              'username': username

Why set scenario only for the last item?

Also, this should check if user is not passing both scenario and result-id - same as for the REST API.

rebased onto 512718df7125af4996034e60132a7e756ba76ad0

3 years ago

rebased onto 41bc9a47d2676d6411970af4350a7a66ffca65e4

3 years ago

Why set scenario only for the last item?

It was actually the last (and only) appended item. I've changed the code to be more clear.

Also, this should check if user is not passing both scenario and result-id - same as for the REST API.

Added

Why not just do 'scenario': scenario in the dict above? Wouldn't passing "scenario": null to the REST API work?

Can you instead add another example, something from real data? Here are results with scenario set: https://taskotron.fedoraproject.org/resultsdb_api/api/v2.0/results?scenario:like=*

rebased onto e5019525aa31cec662fe75beebe05af6f8193fbf

3 years ago

consider using dedent to have a cleaner indentation here.

This seems too long. PEP8 might fail. Could you move subject_identifier to the new line?

Just 2 tiny stylistic checks. Besides that it looks good +1
feel free to merge it once those are addressed.

Can you also update documentation under docs/? Looks like the examples are there listed too.

BTW, documenation is not auto-generated: https://docs.pagure.org/waiverdb/release-notes.html

For me, locally on Fedora 33, building documentation with tox -e docs also doesn't work - it's missing Python packages flask-migrate and stomp.py (dropping them into tox.ini fixes the issue).

This command should fail -- missing subject arguments.

rebased onto 8214f1a70eee178e15f6ded1425b4374ea46a4d7

3 years ago

rebased onto 910eddb9170d3ac70a37c5241f3c1aed7fee1a78

3 years ago

Looks good except a missing test for the new exceptional case.

rebased onto c8e3cc4

3 years ago

Add test for this.

Added

Pull-Request has been merged by vmaljulin

3 years ago