#204 Handling the JSON subject
Closed 2 years ago by ralph. Opened 2 years ago by gnaponie.
gnaponie/waiverdb handle-subject-json  into  master

file modified
+18
@@ -77,6 +77,24 @@ 

      assert result.output == 'Error: The config option "oidc_scopes" is required\n'

  

  

+ def test_malformed_subject(tmpdir):

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

Do we need to repeat this in every test case? Maybe we could factor it out to keep the test cases simpler?

+     p.write("""

+ [waiverdb]

+ auth_method=OIDC

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

+ oidc_id_provider=https://id.stg.fedoraproject.org/openidc/

+ oidc_client_id=waiverdb

+ oidc_scopes=

+     openid

+         """)

+     runner = CliRunner()

+     args = ['-C', p.strpath, '-s', 'subject', '-t', 'testcase', '-c', 'comment']

+     result = runner.invoke(waiverdb_cli, args)

+     assert result.exit_code == 1

+     assert result.output == ('Error: Subject must be a JSON value.\n')

+ 

+ 

  def test_no_product_version(tmpdir):

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

      p.write("""

file modified
+5
@@ -98,6 +98,11 @@ 

          raise click.ClickException('Please specify one subject')

      if not result_ids and not testcase:

          raise click.ClickException('Please specify testcase')

+     if not result_ids and subject and testcase:

+         try:

+             json.loads(subject)

+         except json.JSONDecodeError:

+             raise click.ClickException('Subject must be a JSON value.')

  

      auth_method = config.get('waiverdb', 'auth_method')

      data_list = []

Checking that the subject is actually a JSON, otherwise return an error.

Do we need to repeat this in every test case? Maybe we could factor it out to keep the test cases simpler?

Seems okay to me, although I think it would also be fine to just let the server handle this instead and not bother checking it on the CLI side (as long as we print a useful error in the CLI when the server rejects our request -- maybe there is a separate patch needed for that?)

The CLI should be also changed to accommodate recent changes for the replace 'subject' with 'subject_type' and 'subject_identifier'.

So perhaps replace --subject option (make obsolete) with two new options. Using JSON in CLI is definitely not user-friendly.

maybe we can drop this one?

I'm OK with dropping this and replacing the JSON option on CLI.

Pull-Request has been closed by ralph

2 years ago