Page MenuHomeSoftware Heritage

Fix inconsistencies in exception handling between the base indexers.
ClosedPublic

Authored by vlorentz on Oct 6 2020, 1:31 PM.

Details

Summary
And fix the incorrect test `test_generate_content_get`, which was
passing for the wrong reasons.

Diff Detail

Repository
rDCIDX Metadata indexer
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build has FAILED

Patch application report for D4152 (id=14643)

Rebasing onto 3ac0c155da...

Current branch diff-target is up to date.
Changes applied before test
commit 811175e8765d705bab8783f2835dc2c020c5a4c7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Oct 6 13:30:56 2020 +0200

    Fix inconsistencies in exception handling between the base indexers.

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/63/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/63/console

lgtm

but jenkins disagrees ;)

There is at least one test where the fix is simple (# why? can now go away as well ;), since you fixed the misbehavior, the expectation needs to change:

13:33:08  E         - {'status': 'uneventful'}
13:33:08  E         + {'content_mimetype:add': 2, 'status': 'eventful'}
swh/indexer/indexer.py
537

the initial code could have break here
¯\_(ツ)_/¯

swh/indexer/tests/test_indexer.py
73

jsyk, you can now use the swh_indexer_config fixture now for this.

def test_revision_indexer_catch_exceptions(swh_indexer_config):
    indexer = CrashingIndexer(config=swh_indexer_config)
...
This revision is now accepted and ready to land.Oct 6 2020, 1:52 PM
vlorentz added inline comments.
swh/indexer/indexer.py
537

alternatively: if any(summary_persist.values()): summary["status"] = "eventful", assuming they are all nonnegative

Build is green

Patch application report for D4152 (id=14648)

Could not rebase; Attempt merge onto 3ac0c155da...

Updating 3ac0c15..51b10e8
Fast-forward
 swh/indexer/indexer.py                      | 48 ++++++++--------
 swh/indexer/tests/test_fossology_license.py |  2 +-
 swh/indexer/tests/test_indexer.py           | 85 ++++++++++++++++++++++++++---
 swh/indexer/tests/utils.py                  |  6 +-
 4 files changed, 106 insertions(+), 35 deletions(-)
Changes applied before test
commit 51b10e891d33172e36903bdeb783cdfb960c9bac
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Oct 6 13:30:56 2020 +0200

    Fix inconsistencies in exception handling between the base indexers.
    
    And fix the incorrect test `test_generate_content_get`, which was
    passing for the wrong reasons.

commit 6535444922ebf2adc6182a0a54e0e9e1c3ce3cdc
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Oct 6 13:58:05 2020 +0200

    test_generate_content_get_no_result: Pass a multiple of 2 as number of partitions.
    
    This caused indexers to crash, but they caught the error and returned
    uneventful instead.
    A future commit will remove this exception silencing, so it will
    make this test fail.

commit 337ac3a79be87269d510b79b34c4fb981fea41e6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Oct 6 13:56:52 2020 +0200

    Fix invalid None value for the list of licenses in tests.
    
    This caused indexers to crash, but they caught the error and returned
    uneventful instead.
    A future commit will remove this exception silencing, so it will
    make tests fail.

See https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/66/ for more details.