Details

Reviewers

Group Reviewers

Commits

rDCIDXd3ec22d98aee: linearize assert_results_ok.
rDCIDX0b5588346964: Make tests actually run.

Summary

assertions in 'for indexed_data in actual_results:' never ran, because
'actual_results' was empty.

This commit creates two test failures, that are caused by an already
existing bug in the ctags indexer that I will fix later.

Diff Detail

Repository

rDCIDX Metadata indexer

Branch

fix-noop-tests

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 3245
Build 4171: tox-on-jenkins	Jenkins
Build 4170: arc lint + arc unit

Event Timeline

vlorentz created this revision.Dec 20 2018, 7:55 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 20 2018, 7:55 PM

remove debugging statement.

vlorentz added a child revision: D874: ContentIndexer: convert hash to bytes..Dec 20 2018, 7:56 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/164/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/164/console

Harbormaster failed remote builds in B3232: Diff 2781!Dec 20 2018, 7:58 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/165/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/165/console

Harbormaster failed remote builds in B3233: Diff 2782!Dec 20 2018, 8:01 PM

vlorentz added a child revision: D877: Fix ctags tests..Dec 21 2018, 11:20 AM

The control flow of the assert_results_ok function is really hard to follow.

It'd be nicer to have two functions, one for the current format and another for the legacy format (assert_legacy_results_ok).

This would also have the side effect of properly testing whether the results are in the expected (old or new) format.

WDYT?

swh/indexer/tests/test_utils.py
503	What's the task reference for the update to the new format?

In D876#18785, @olasd wrote:

The control flow of the assert_results_ok function is really hard to follow.

It'd be nicer to have two functions, one for the current format and another for the legacy format (assert_legacy_results_ok).

This would also have the side effect of properly testing whether the results are in the expected (old or new) format.

WDYT?

agreed. will do

swh/indexer/tests/test_utils.py
503	T1433

vlorentz planned changes to this revision.Dec 21 2018, 3:34 PM

olasd added inline comments.Dec 21 2018, 3:34 PM

swh/indexer/tests/test_utils.py
503	I meant to reference it in the comment ;)

vlorentz mentioned this in D874: ContentIndexer: convert hash to bytes..Dec 21 2018, 3:46 PM

linearize assert_results_ok.

vlorentz marked 2 inline comments as done.Dec 21 2018, 3:55 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/171/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/171/console

Harbormaster failed remote builds in B3243: Diff 2793!Dec 21 2018, 3:57 PM

olasd added inline comments.Dec 21 2018, 4:02 PM

swh/indexer/tests/test_utils.py
478–483	If you want to go that way, I'd rather make that attribute default to False, leaving legacy indexer test classes override it, than making it an abstract property with a hack. Did I ever tell you how I hate pretty much everything about abc? :-)