#4070 api unit test
Closed by tkopecek. Opened by mikem.
mikem/koji api-test  into  master

Download 4070.patch

For discussion. This test/tool attempts to keep us aware of any api changes that we make.

The devtools/check-api script when run without options will output json data that represents a targeted portion of koji's api. If given an existing json file, it will instead compare the api to that data and report problems. There is also a unit test that runs the script against the api from 1.34.

Current output is:

[mikem@localhost koji]$ devtools/check-api tests/test_api/data/api.json
Added module global: koji.DateTime
Added module global: koji.tasks.RefuseTask
Added module global: koji.xmlrpcplus.defusedxmlrpc
Added module global: koji.xmlrpcplus.xmlrpc_server
Signature changed for rpc call host.updateHost:
  old: (task_load, ready)
  new: (task_load, ready, data=None)
Args extended for rpc call host.updateHost: ['data']
Signature changed for rpc call listUsers:
  old: (userType=0, prefix=None, queryOpts=None)
  new: (userType=0, prefix=None, queryOpts=None, perm=None, inherited_perm=False)
Args extended for rpc call listUsers: ['perm', 'inherited_perm']
Signature changed for rpc call scheduler.getLogMessages:
  old: (clauses=None, fields=None)
  new: (clauses=None, fields=None, opts=None)
Args extended for rpc call scheduler.getLogMessages: ['opts']
Signature changed for rpc call scheduler.getTaskRuns:
  old: (clauses=None, fields=None)
  new: (clauses=None, fields=None, opts=None)
Args extended for rpc call scheduler.getTaskRuns: ['opts']
Got 12 warnings and 0 errors

Without the included fix, we would also see this error:

ERROR: Dropped module global: koji.xmlrpc_client

Metadata Update from @mikem:
- Pull-request tagged with: discussion, no_qe

3 new commits added

  • update api data
  • a bit more filtering
  • also check class methods

Comparing against 1.33 reports:

ERROR: Dropped module global: koji.AUTHTYPE_GSSAPI
ERROR: Dropped module global: koji.AUTHTYPE_KERB
ERROR: Dropped module global: koji.AUTHTYPE_NORMAL
ERROR: Dropped module global: koji.AUTHTYPE_SSL
ERROR: Dropped module global: koji.get_sequence_value

get_sequence_value() was legitimately deprecated (#3817) and notably out of place in the lib. Distinguishing such deprecations is an outstanding todo item here. At worst, this could be done by manually editing the api data.

Dropping the AUTHTYPE values, otoh, should not have been done. There's no good way to warn users about such a change and maintaining the existing values as aliases (as we did for repo states in the past) is a very minor burden. My hope is that this test will help us avoid such unnecessary api breakage in the future.

3 new commits added

  • flake8
  • note modules and imported globals in error
  • handle koji versions back to 1.18

@tkopecek any thoughts?

Yes, I believe it is is a good piece of tests. We should run it as a part of basic "make test3", so possible errors are either argued and changed in the json or reverted.

For this concrete case it was my explicit ack for removal of authtypes, so it wouldn't help much. Possibly only solution is to request two reviewers for evert PR. Not sure if it is worth given our time constaints :-(

What could make sense is to request two reviewers in every case when API (lib and/or hub) is being changed.

Additional review for api changes is certainly warranted.

I would also say that that reviewers should note when changes are very invasive or at risk of introducing incompatibilities and ask for additional eyes on the code as needed.

We should run it as a part of basic "make test3"

This PR includes a unit test that does this — tests/test_api/test_api.py

If this seems ok, maybe we can go ahead and pull it in for 1.35 as is. We can always improve it later.

(probable) merged without pg tool, so it is not autoclosed. Relevat commit https://pagure.io/koji/c/00b116354436b9a8acc8dee523c3aaa000fb0305?branch=master

Pull-Request has been closed by tkopecek

Metadata