irods / python-irodsclient

A Python API for iRODS
Other
62 stars 73 forks source link

test PRC's consistency with iRODS server in mapping GenQuery1 column names to numeric keys #647

Closed d-w-moore closed 1 week ago

d-w-moore commented 1 month ago

What still remains is to diff the full set against rodsGenQuery.h in a unit test

trel commented 1 month ago

i see a replacement of six in here that ... should have been in the other PR?

d-w-moore commented 1 month ago

i see a replacement of six in here that ... should have been in the other PR?

Try reloading.

trel commented 1 month ago

diff is against main, so I suppose that's why it's showing here. If the other PR is merged first, this diff will show differently.

Carry on.

d-w-moore commented 1 month ago

diff is against main, so I suppose that's why it's showing here. If the other PR is merged first, this diff will show differently.

Carry on.

You're right though, there is a six dependency in there. Good catch @trel . Missed that....

trel commented 1 month ago

let's get the six removals done in https://github.com/irods/python-irodsclient/pull/640 - keep them all together.

d-w-moore commented 3 weeks ago

Running the 643 test on a reversion of the column corrections produces this, which is ... good.

$ python -m unittest irods.test.meta_test.TestMeta.test_that_all_column_mappings_are_uniquely_and_properly_defined__issue_643
F
======================================================================
FAIL: test_that_all_column_mappings_are_uniquely_and_properly_defined__issue_643 (irods.test.meta_test.TestMeta)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/daniel/ppy3/python-irodsclient/irods/test/meta_test.py", line 626, in test_that_all_column_mappings_are_uniquely_and_properly_defined__issue_643
    self.assertEqual( sr, allowed_outliers )
AssertionError: Items in the first set but not the second:
'COL_META_COLL_ATTR_UNITS'
'COL_TICKET_ALLOWED_GROUP'
'COL_TICKET_ALLOWED_USER'
'COL_META_RESC_ATTR_UNITS'

----------------------------------------------------------------------
Ran 1 test in 0.135s

FAILED (failures=1)
d-w-moore commented 3 weeks ago

Looking good.

What's left for this PR? Are we almost ready for merging?

Yes I think we're pretty much there, upon comments resolution.

d-w-moore commented 2 weeks ago

Looking good.

What's left for this PR? Are we almost ready for merging?

I think it is ready, yes. Need a squash.

d-w-moore commented 2 weeks ago

This has now had some reasonable squashing done.

trel commented 2 weeks ago

parentheses don't add anything in my opinion.

d-w-moore commented 2 weeks ago

parentheses don't add anything in my opinion.

Will look&correct

d-w-moore commented 2 weeks ago

Done, no paren-veloping. Assuming there is approval, do we want more squashing?

trel commented 2 weeks ago

i vote for them being separate (as is) - easier to follow the fix and the tests.

d-w-moore commented 1 week ago

I agree. Keeping things as-is is good.

Pound it.

Done!