Page MenuHomeSoftware Heritage
Differential D6133 Diff 23788 swh/lister/maven/tests/test_lister.py

Changeset View

Standalone View

View Options

swh/lister/maven/tests/test_lister.py

  • This file was added.
# Copyright (C) 2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information
from pathlib import Path
import pytest
import requests
from swh.lister.maven.lister import MavenLister
MVN_URL = "https://repo1.maven.org/maven2/" # main maven repo url
INDEX_URL = "http://indexes/export.fld" # index directory url
URL_POM_1 = MVN_URL + "al/aldi/sprova4j/0.1.0/sprova4j-0.1.0.pom"
URL_POM_2 = MVN_URL + "al/aldi/sprova4j/0.1.1/sprova4j-0.1.1.pom"
URL_POM_3 = MVN_URL + "com/arangodb/arangodb-graphql/1.2/arangodb-graphql-1.2.pom"
LIST_GIT = (
"git://github.com/aldialimucaj/sprova4j.git",
"https://github.com/aldialimucaj/sprova4j.git",
)
LIST_GIT_INCR = ("git://github.com/ArangoDB-Community/arangodb-graphql-java.git",)
LIST_SRC = (
MVN_URL + "al/aldi/sprova4j/0.1.0/sprova4j-0.1.0-sources.jar",
MVN_URL + "al/aldi/sprova4j/0.1.1/sprova4j-0.1.1-sources.jar",
)
LIST_SRC_DATA = (
{
"type": "maven",
"url": "https://repo1.maven.org/maven2/al/aldi/sprova4j"
+ "/0.1.0/sprova4j-0.1.0-sources.jar",
"time": 1626109619335,
"gid": "al.aldi",
"aid": "sprova4j",
"version": "0.1.0",
},
{
"type": "maven",
"url": "https://repo1.maven.org/maven2/al/aldi/sprova4j"
+ "/0.1.1/sprova4j-0.1.1-sources.jar",
"time": 1626111425534,
"gid": "al.aldi",
"aid": "sprova4j",
"version": "0.1.1",
},
)
@pytest.fixture
def maven_index(datadir) -> str:
text = Path(datadir, "http_indexes", "export.fld").read_text()
return text
@pytest.fixture
def maven_index_incr(datadir) -> str:
text = Path(datadir, "http_indexes", "export_incr.fld").read_text()
return text
@pytest.fixture
def maven_pom_1(datadir) -> str:
text = Path(datadir, "https_maven.org", "sprova4j-0.1.0.pom").read_text()
douarddaUnsubmitted
Not Done
Inline Actions

Is it really necessary for these functions to be fixtures?

douardda: Is it really necessary for these functions to be fixtures?
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

I'm quite new to python fixtures, but as I understand it I'd say yes. The script is expected to http-get these files, so fixtures are the right way to mock it, right? That's what I've seen on other listers too.

Is there a better way to do it?

borisbaldassari: I'm quite new to python fixtures, but as I understand it I'd say yes. The script is expected to…
return text
@pytest.fixture
douarddaUnsubmitted
Done
Inline Actions

The "two collections" is not clear to me.

douardda: The "two collections" is not clear to me.
def maven_pom_2(datadir) -> str:
text = Path(datadir, "https_maven.org", "sprova4j-0.1.1.pom").read_text()
return text
@pytest.fixture
def maven_pom_3(datadir) -> str:
text = Path(datadir, "https_maven.org", "arangodb-graphql-1.2.pom").read_text()
return text
douarddaUnsubmitted
Done
Inline Actions

why not just assert sorted_lister_urls == sorted_scheduler_origins ?
Assuming you build sorted_scheduler_origins as sorted(x.url for x in scheduler_origins) I mean.

Also the list in list(sorted(x)) is not necessary I believe.

douardda: why not just `assert sorted_lister_urls == sorted_scheduler_origins` ? Assuming you build…
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Well spotted, thanks. I removed the function and did an assert == on sorted lists. And renamed the variable, you're once again definitely right. Thanks!

borisbaldassari: Well spotted, thanks. I removed the function and did an assert == on sorted lists. And renamed…
def test_maven_full_listing(
swh_scheduler, requests_mock, mocker, maven_index, maven_pom_1, maven_pom_2,
):
"""Covers full listing of multiple pages, checking page results and listed
origins, statelessness."""
vlorentzUnsubmitted
Done
Inline Actions

this comment doesn't seem accurate

vlorentz: this comment doesn't seem accurate
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Good point. Removed rate-limit and page size. rate-limit is tested below, and page size doesn't make sense (only one yield per page).

borisbaldassari: Good point. Removed rate-limit and page size. rate-limit is tested below, and page size doesn't…
lister = MavenLister(
scheduler=swh_scheduler,
url=MVN_URL,
instance="maven.org",
index_url=INDEX_URL,
incremental=False,
)
douarddaUnsubmitted
Done
Inline Actions

Why these local variables? They look unnecessary to me.

douardda: Why these local variables? They look unnecessary to me.
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Agreed. Were there once for readability, not relevant anymore. Thanks!

borisbaldassari: Agreed. Were there once for readability, not relevant anymore. Thanks!
# Set up test.
index_text = maven_index
requests_mock.get(INDEX_URL, text=index_text)
requests_mock.get(URL_POM_1, text=maven_pom_1)
requests_mock.get(URL_POM_2, text=maven_pom_2)
# Then run the lister.
stats = lister.run()
# Start test checks.
assert stats.pages == 4
assert stats.origins == 4
scheduler_origins = swh_scheduler.get_listed_origins(lister.lister_obj.id).results
origin_urls = [origin.url for origin in scheduler_origins]
douarddaUnsubmitted
Done
Inline Actions

It's generally not a good idea to shadow a builtin function/type (list here). Better choose another variable name. Especially since it looks to be a dictionary...

douardda: It's generally not a good idea to shadow a builtin function/type (`list` here). Better choose…
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Very good point. Changed to src. Thanks!

borisbaldassari: Very good point. Changed to src. Thanks!
assert sorted(origin_urls) == sorted(LIST_GIT + LIST_SRC)
for origin in scheduler_origins:
if origin.visit_type == "maven":
for src in LIST_SRC_DATA:
if src.get("url") == origin.url:
artifact = origin.extra_loader_arguments["artifacts"][0]
assert src.get("time") == artifact["time"]
assert src.get("gid") == artifact["gid"]
assert src.get("aid") == artifact["aid"]
assert src.get("version") == artifact["version"]
break
else:
raise AssertionError
scheduler_state = lister.get_state_from_scheduler()
assert scheduler_state is not None
assert scheduler_state.last_seen_doc == -1
douarddaUnsubmitted
Done
Inline Actions
if list.get("url") == s.url:
- assert (
- list.get("time")
- == s.extra_loader_arguments["artifacts"][0]["time"]
- )
- assert (
- list.get("gid")
- == s.extra_loader_arguments["artifacts"][0]["gid"]
- )
- assert (
- list.get("aid")
- == s.extra_loader_arguments["artifacts"][0]["aid"]
- )
- assert (
- list.get("version")
- == s.extra_loader_arguments["artifacts"][0]["version"]
- )
+ artifact = s.extra_loader_arguments["artifacts"][0]
+ assert list.get("time") == artifact["time"]
+ assert list.get("gid") == artifact["gid"]
+ assert list.get("aid") == artifact["aid"]
+ assert list.get("version") == artifact["version"]
break
douardda:
assert scheduler_state.last_seen_pom == -1
vlorentzUnsubmitted
Done
Inline Actions

Wouldn't it be easier to test this in check_listed_origins, as it zips the two lists?

vlorentz: Wouldn't it be easier to test this in check_listed_origins, as it zips the two lists?
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Well, that depends on the previous comment. As it is right now, only origins of type jar should have the metadata. Postponing the answer until the discussion on metadata is settled.

borisbaldassari: Well, that depends on the previous comment. As it is right now, only origins of type jar should…
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Refactored.

borisbaldassari: Refactored.
def test_maven_incremental_listing(
vlorentzUnsubmitted
Done
Inline Actions
== s.extra_loader_arguments["artifacts"][0]["version"]
)
- found = True
- break
- assert found
+ else:
+ assert False, "not found"
assert lister.get_state_from_scheduler() is None

btw, for loops in Python can take an else clause like this

vlorentz: btw, `for` loops in Python can take an `else` clause like this
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Wouah! I'm new to python, but start really liking it.. Thanks! ;-)

borisbaldassari: Wouah! I'm new to python, but start really liking it.. Thanks! ;-)
swh_scheduler,
requests_mock,
mocker,
maven_index,
maven_index_incr,
maven_pom_1,
maven_pom_2,
maven_pom_3,
):
"""Covers full listing of multiple pages, checking page results and listed
origins, with a second updated run for statefulness."""
lister = MavenLister(
scheduler=swh_scheduler,
douarddaUnsubmitted
Done
Inline Actions

please explain which HTTP error this is actually testing.

douardda: please explain which HTTP error this is actually testing.
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Added documentation and more tests:

  • http error on maven_index not found,
  • added error 404 error code.
borisbaldassari: Added documentation and more tests: * http error on maven_index not found, * added error 404…
url=MVN_URL,
instance="maven.org",
index_url=INDEX_URL,
incremental=True,
douarddaUnsubmitted
Done
Inline Actions

I don't get why we have 2 listed origins if there have been HTTP error in the process. If it's expected, it needs a bit of explanation.

douardda: I don't get why we have 2 listed origins if there have been HTTP error in the process. If it's…
borisbaldassariAuthorUnsubmitted
Done
Inline Actions

Right. Yes, it's expected. Added this comment:

  1. If the maven_index step succeeded but not the get_pom step,
  2. then we get only the 2 jar origins (and not the 2 additional
  3. src origins).
borisbaldassari: Right. Yes, it's expected. Added this comment: # If the maven_index step succeeded but not the…
)
# Set up test.
requests_mock.get(INDEX_URL, text=maven_index)
requests_mock.get(URL_POM_1, text=maven_pom_1)
requests_mock.get(URL_POM_2, text=maven_pom_2)
# Then run the lister.
stats = lister.run()
# Start test checks.
assert lister.incremental
assert lister.updated
assert stats.pages == 4
assert stats.origins == 4
# Second execution of the lister, incremental mode
lister = MavenLister(
scheduler=swh_scheduler,
url=MVN_URL,
instance="maven.org",
index_url=INDEX_URL,
incremental=True,
)
scheduler_state = lister.get_state_from_scheduler()
assert scheduler_state is not None
assert scheduler_state.last_seen_doc == 3
assert scheduler_state.last_seen_pom == 3
# Set up test.
requests_mock.get(INDEX_URL, text=maven_index_incr)
requests_mock.get(URL_POM_3, text=maven_pom_3)
# Then run the lister.
stats = lister.run()
# Start test checks.
assert lister.incremental
assert lister.updated
assert stats.pages == 1
assert stats.origins == 1
scheduler_origins = swh_scheduler.get_listed_origins(lister.lister_obj.id).results
origin_urls = [origin.url for origin in scheduler_origins]
assert sorted(origin_urls) == sorted(LIST_SRC + LIST_GIT + LIST_GIT_INCR)
for origin in scheduler_origins:
if origin.visit_type == "maven":
for src in LIST_SRC_DATA:
if src.get("url") == origin.url:
artifact = origin.extra_loader_arguments["artifacts"][0]
assert src.get("time") == artifact["time"]
assert src.get("gid") == artifact["gid"]
assert src.get("aid") == artifact["aid"]
assert src.get("version") == artifact["version"]
break
else:
raise AssertionError
scheduler_state = lister.get_state_from_scheduler()
assert scheduler_state is not None
assert scheduler_state.last_seen_doc == 4
assert scheduler_state.last_seen_pom == 4
@pytest.mark.parametrize("http_code", [400, 404, 500, 502])
def test_maven_list_http_error(
swh_scheduler, requests_mock, mocker, maven_index, http_code
):
"""Test handling of some common HTTP errors:
- 400: Bad request.
- 404: Resource no found.
- 500: Internal server error.
- 502: Bad gateway ou proxy Error.
"""
lister = MavenLister(scheduler=swh_scheduler, url=MVN_URL, index_url=INDEX_URL)
# Test failure of index retrieval.
requests_mock.get(INDEX_URL, status_code=http_code)
with pytest.raises(requests.HTTPError):
lister.run()
# Test failure of artefacts retrieval.
requests_mock.get(INDEX_URL, text=maven_index)
requests_mock.get(URL_POM_1, status_code=http_code)
with pytest.raises(requests.HTTPError):
lister.run()
# If the maven_index step succeeded but not the get_pom step,
# then we get only the 2 maven-jar origins (and not the 2 additional
# src origins).
scheduler_origins = swh_scheduler.get_listed_origins(lister.lister_obj.id).results
assert len(scheduler_origins) == 2
About · Developer information · Legal