Page MenuHomeSoftware Heritage

Make tests actually run.
ClosedPublic

Authored by vlorentz on Thu, Dec 20, 7:55 PM.

Details

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 Object indexer
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Thu, Dec 20, 7:55 PM
vlorentz updated this revision to Diff 2782.Thu, Dec 20, 7:56 PM
  • remove debugging statement.
olasd added a subscriber: olasd.Fri, Dec 21, 3:31 PM

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
495

What's the task reference for the update to the new format?

vlorentz marked an inline comment as done.Fri, Dec 21, 3:34 PM
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
495
vlorentz planned changes to this revision.Fri, Dec 21, 3:34 PM
olasd added inline comments.Fri, Dec 21, 3:34 PM
swh/indexer/tests/test_utils.py
495

I meant to reference it in the comment ;)

vlorentz updated this revision to Diff 2793.Fri, Dec 21, 3:54 PM
  • linearize assert_results_ok.
vlorentz marked 2 inline comments as done.Fri, Dec 21, 3:55 PM
olasd added inline comments.Fri, Dec 21, 4:02 PM
swh/indexer/tests/test_utils.py
477–478

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? :-)

vlorentz updated this revision to Diff 2795.Fri, Dec 21, 4:06 PM
  • Default value for legacy_get_format.
olasd accepted this revision.Fri, Dec 21, 4:33 PM
This revision is now accepted and ready to land.Fri, Dec 21, 4:33 PM
vlorentz updated this revision to Diff 2801.Mon, Jan 7, 11:17 AM
  • rebase
This revision was automatically updated to reflect the committed changes.