From b7ec6cb120b4c2a5f2fb3153794a4c0f4fb97135 Mon Sep 17 00:00:00 2001 From: Valentin Lorentz Date: Wed, 24 Aug 2022 17:21:24 +0200 Subject: [PATCH] tests: Simplify origin comparison and improve pytest diff on failure By using a single equality instead of checking len() then zip() to check one by one, pytest can find the common/missing elements and print them nicely when the two lists are unequal. --- swh/lister/arch/tests/test_lister.py | 22 +++------ swh/lister/aur/tests/test_lister.py | 18 +++---- swh/lister/bitbucket/tests/test_lister.py | 12 ++--- swh/lister/crates/tests/test_lister.py | 60 ++++++++++++----------- swh/lister/gitea/tests/test_lister.py | 9 +--- swh/lister/gogs/tests/test_lister.py | 9 +--- swh/lister/pypi/tests/test_lister.py | 9 +--- swh/lister/tuleap/tests/test_lister.py | 8 +-- 8 files changed, 53 insertions(+), 94 deletions(-) diff --git a/swh/lister/arch/tests/test_lister.py b/swh/lister/arch/tests/test_lister.py index abe8e81..daa8712 100644 --- a/swh/lister/arch/tests/test_lister.py +++ b/swh/lister/arch/tests/test_lister.py @@ -1373,28 +1373,22 @@ def test_arch_lister(datadir, requests_mock_datadir, swh_scheduler): assert res.pages == 9 assert res.origins == 12 - expected_origins_sorted = sorted(expected_origins, key=lambda x: x.get("url")) - scheduler_origins_sorted = sorted( - swh_scheduler.get_listed_origins(lister.lister_obj.id).results, - key=lambda x: x.url, - ) - - assert len(scheduler_origins_sorted) == len(expected_origins_sorted) + scheduler_origins = swh_scheduler.get_listed_origins(lister.lister_obj.id).results assert [ ( scheduled.visit_type, scheduled.url, - scheduled.extra_loader_arguments.get("artifacts"), - scheduled.extra_loader_arguments.get("arch_metadata"), + scheduled.extra_loader_arguments["artifacts"], + scheduled.extra_loader_arguments["arch_metadata"], ) - for scheduled in scheduler_origins_sorted + for scheduled in sorted(scheduler_origins, key=lambda scheduled: scheduled.url) ] == [ ( "arch", - expected.get("url"), - expected.get("extra_loader_arguments").get("artifacts"), - expected.get("extra_loader_arguments").get("arch_metadata"), + expected["url"], + expected["extra_loader_arguments"]["artifacts"], + expected["extra_loader_arguments"]["arch_metadata"], ) - for expected in expected_origins_sorted + for expected in sorted(expected_origins, key=lambda expected: expected["url"]) ] diff --git a/swh/lister/aur/tests/test_lister.py b/swh/lister/aur/tests/test_lister.py index c403dad..46c08da 100644 --- a/swh/lister/aur/tests/test_lister.py +++ b/swh/lister/aur/tests/test_lister.py @@ -99,28 +99,22 @@ def test_aur_lister(datadir, requests_mock_datadir, swh_scheduler): assert res.pages == 4 assert res.origins == 4 - scheduler_origins_sorted = sorted( - swh_scheduler.get_listed_origins(lister.lister_obj.id).results, - key=lambda x: x.url, - ) - expected_origins_sorted = sorted(expected_origins, key=lambda x: x.get("url")) - - assert len(scheduler_origins_sorted) == len(expected_origins_sorted) + scheduler_origins = swh_scheduler.get_listed_origins(lister.lister_obj.id).results assert [ ( scheduled.visit_type, scheduled.url, - scheduled.extra_loader_arguments.get("artifacts"), + scheduled.extra_loader_arguments["artifacts"], ) - for scheduled in scheduler_origins_sorted + for scheduled in sorted(scheduler_origins, key=lambda scheduled: scheduled.url) ] == [ ( "aur", - expected.get("url"), - expected.get("extra_loader_arguments").get("artifacts"), + expected["url"], + expected["extra_loader_arguments"]["artifacts"], ) - for expected in expected_origins_sorted + for expected in sorted(expected_origins, key=lambda expected: expected["url"]) ] diff --git a/swh/lister/bitbucket/tests/test_lister.py b/swh/lister/bitbucket/tests/test_lister.py index c568dbf..e624e8e 100644 --- a/swh/lister/bitbucket/tests/test_lister.py +++ b/swh/lister/bitbucket/tests/test_lister.py @@ -29,15 +29,9 @@ def bb_api_repositories_page2(datadir): def _check_listed_origins(lister_origins, scheduler_origins): """Asserts that the two collections have the same origins from the point of view of the lister""" - - sorted_lister_origins = list(sorted(lister_origins)) - sorted_scheduler_origins = list(sorted(scheduler_origins)) - - assert len(sorted_lister_origins) == len(sorted_scheduler_origins) - - for lo, so in zip(sorted_lister_origins, sorted_scheduler_origins): - assert lo.url == so.url - assert lo.last_update == so.last_update + assert {(lo.url, lo.last_update) for lo in lister_origins} == { + (so.url, so.last_update) for so in scheduler_origins + } def test_bitbucket_incremental_lister( diff --git a/swh/lister/crates/tests/test_lister.py b/swh/lister/crates/tests/test_lister.py index 2c62449..8b26379 100644 --- a/swh/lister/crates/tests/test_lister.py +++ b/swh/lister/crates/tests/test_lister.py @@ -137,20 +137,22 @@ def test_crates_lister(datadir, tmp_path, swh_scheduler): assert res.pages == 3 assert res.origins == 3 - expected_origins_sorted = sorted(expected_origins, key=lambda x: x.get("url")) - scheduler_origins_sorted = sorted( - swh_scheduler.get_listed_origins(lister.lister_obj.id).results, - key=lambda x: x.url, - ) - - for scheduled, expected in zip(scheduler_origins_sorted, expected_origins_sorted): - assert scheduled.visit_type == "crates" - assert scheduled.url == expected.get("url") - assert scheduled.extra_loader_arguments.get("artifacts") == expected.get( - "artifacts" + scheduler_origins = swh_scheduler.get_listed_origins(lister.lister_obj.id).results + assert [ + ( + scheduled.visit_type, + scheduled.url, + scheduled.extra_loader_arguments["artifacts"], ) - - assert len(scheduler_origins_sorted) == len(expected_origins_sorted) + for scheduled in sorted(scheduler_origins, key=lambda scheduled: scheduled.url) + ] == [ + ( + "crates", + expected["url"], + expected["artifacts"], + ) + for expected in sorted(expected_origins, key=lambda expected: expected["url"]) + ] def test_crates_lister_incremental(datadir, tmp_path, swh_scheduler): @@ -176,22 +178,24 @@ def test_crates_lister_incremental(datadir, tmp_path, swh_scheduler): assert res.pages == 2 assert res.origins == 2 - expected_origins_sorted = sorted( - expected_origins_incremental, key=lambda x: x.get("url") - ) - scheduler_origins_sorted = sorted( - swh_scheduler.get_listed_origins(lister.lister_obj.id).results, - key=lambda x: x.url, - ) - - for scheduled, expected in zip(scheduler_origins_sorted, expected_origins_sorted): - assert scheduled.visit_type == "crates" - assert scheduled.url == expected.get("url") - assert scheduled.extra_loader_arguments.get("artifacts") == expected.get( - "artifacts" + scheduler_origins = swh_scheduler.get_listed_origins(lister.lister_obj.id).results + assert [ + ( + scheduled.visit_type, + scheduled.url, + scheduled.extra_loader_arguments["artifacts"], ) - - assert len(scheduler_origins_sorted) == len(expected_origins_sorted) + for scheduled in sorted(scheduler_origins, key=lambda scheduled: scheduled.url) + ] == [ + ( + "crates", + expected["url"], + expected["artifacts"], + ) + for expected in sorted( + expected_origins_incremental, key=lambda expected: expected["url"] + ) + ] def test_crates_lister_incremental_nothing_new(datadir, tmp_path, swh_scheduler): diff --git a/swh/lister/gitea/tests/test_lister.py b/swh/lister/gitea/tests/test_lister.py index 08a17b5..90ec624 100644 --- a/swh/lister/gitea/tests/test_lister.py +++ b/swh/lister/gitea/tests/test_lister.py @@ -51,14 +51,7 @@ def check_listed_origins(lister_urls: List[str], scheduler_origins: List[ListedO """Asserts that the two collections have the same origin URLs. Does not test last_update.""" - - sorted_lister_urls = list(sorted(lister_urls)) - sorted_scheduler_origins = list(sorted(scheduler_origins)) - - assert len(sorted_lister_urls) == len(sorted_scheduler_origins) - - for l_url, s_origin in zip(sorted_lister_urls, sorted_scheduler_origins): - assert l_url == s_origin.url + assert set(lister_urls) == {origin.url for origin in scheduler_origins} def test_gitea_full_listing( diff --git a/swh/lister/gogs/tests/test_lister.py b/swh/lister/gogs/tests/test_lister.py index b9f48a9..5c9b651 100644 --- a/swh/lister/gogs/tests/test_lister.py +++ b/swh/lister/gogs/tests/test_lister.py @@ -96,14 +96,7 @@ def check_listed_origins(lister_urls: List[str], scheduler_origins: List[ListedO """Asserts that the two collections have the same origin URLs. Does not test last_update.""" - - sorted_lister_urls = list(sorted(lister_urls)) - sorted_scheduler_origins = list(sorted(scheduler_origins)) - - assert len(sorted_lister_urls) == len(sorted_scheduler_origins) - - for l_url, s_origin in zip(sorted_lister_urls, sorted_scheduler_origins): - assert l_url == s_origin.url + assert set(lister_urls) == {origin.url for origin in scheduler_origins} def test_gogs_full_listing( diff --git a/swh/lister/pypi/tests/test_lister.py b/swh/lister/pypi/tests/test_lister.py index fefb01f..a6dac88 100644 --- a/swh/lister/pypi/tests/test_lister.py +++ b/swh/lister/pypi/tests/test_lister.py @@ -16,14 +16,7 @@ from swh.scheduler.model import ListedOrigin def check_listed_origins(lister_urls: List[str], scheduler_origins: List[ListedOrigin]): """Asserts that the two collections have the same origin URLs""" - - sorted_lister_urls = list(sorted(lister_urls)) - sorted_scheduler_origins = list(sorted(scheduler_origins)) - - assert len(sorted_lister_urls) == len(sorted_scheduler_origins) - - for l_url, s_origin in zip(sorted_lister_urls, sorted_scheduler_origins): - assert l_url == s_origin.url + assert set(lister_urls) == {origin.url for origin in scheduler_origins} @pytest.mark.parametrize( diff --git a/swh/lister/tuleap/tests/test_lister.py b/swh/lister/tuleap/tests/test_lister.py index 5e74d35..16d0c7a 100644 --- a/swh/lister/tuleap/tests/test_lister.py +++ b/swh/lister/tuleap/tests/test_lister.py @@ -97,13 +97,7 @@ def check_listed_origins(lister_urls: List[str], scheduler_origins: List[ListedO Does not test last_update.""" - sorted_lister_urls = list(sorted(lister_urls)) - sorted_scheduler_origins = list(sorted(scheduler_origins)) - - assert len(sorted_lister_urls) == len(sorted_scheduler_origins) - - for l_url, s_origin in zip(sorted_lister_urls, sorted_scheduler_origins): - assert l_url == s_origin.url + assert set(lister_urls) == {origin.url for origin in scheduler_origins} def test_tuleap_full_listing(