Page MenuHomeSoftware Heritage
Differential D3957

config: Clarify configuration setup client code
ClosedPublic
Actions

Authored by ardumont on Sep 15 2020, 6:36 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T1532: Cleanup deprecated configuration code in swh modules
Commits
rDDEPe5eacbbf8b3a: config: Clarify configuration setup path for the client code
Summary

Second part of getting rid of the SWHConfig mixin.
Related to client code now

Related to T1532

Test Plan

tox

Diff Detail

Repository
rDDEP Push deposit
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15203
Build 23439: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 23438: arc lint + arc unit

Event Timeline

ardumont created this revision.Sep 15 2020, 6:36 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 15 2020, 6:36 PM
swh-public-ci added a comment.Sep 15 2020, 6:40 PM

Build is green

Patch application report for D3957 (id=13936)

Could not rebase; Attempt merge onto f41a5a6c06...

Updating f41a5a6c..05a4a5f9
Fast-forward
 requirements-swh.txt                           |  2 +-
 swh/deposit/client.py                          | 17 ++-----
 swh/deposit/config.py                          | 47 ++++++++-----------
 swh/deposit/loader/checker.py                  | 26 ++++-------
 swh/deposit/tests/api/test_deposit_schedule.py | 18 +++-----
 swh/deposit/tests/conftest.py                  | 64 +++++++++++++-------------
 swh/deposit/tests/loader/conftest.py           | 37 ++++-----------
 swh/deposit/tests/loader/test_checker.py       |  8 ++--
 swh/deposit/tests/loader/test_tasks.py         | 12 ++---
 9 files changed, 91 insertions(+), 140 deletions(-)
Changes applied before test
commit 05a4a5f9685ec157e7d6aa54e5591d56b3450044
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 15 18:34:38 2020 +0200

    config: Clarify configuration setup client code
    
    Related to T1532

commit fbd84db9424b8f8b2ea102c5f84ef04b78d90b8c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 15 10:51:37 2020 +0200

    config: Clarify configuration setup for server code
    
    Mixin SWHConfig is deprecated. This unplugs the inheritance to that class which
    has becomes too much complicated to follow.
    
    Related to T1532

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

Harbormaster completed remote builds in B15147: Diff 13936.Sep 15 2020, 6:40 PM
vlorentz added a subscriber: vlorentz.Sep 16 2020, 9:26 AM
vlorentz added inline comments.
swh/deposit/tests/loader/conftest.py
24–25

how does the deposit know how to connect to pg in tests, now?

ardumont added inline comments.Sep 16 2020, 4:16 PM
swh/deposit/tests/loader/conftest.py
24–25

The tests in the "loader" folder don't connect to the db.
Thus why they pass here.

They are passing through the request_mock_datadir fixture and other mocker things.

Note: Historically called "loader/" but it used to be a loader in here, remains only the checker now.

vlorentz accepted this revision.Sep 16 2020, 4:23 PM
This revision is now accepted and ready to land.Sep 16 2020, 4:23 PM
ardumont mentioned this in D3968: conf/deposit: Explicit deposit api configuration.Sep 16 2020, 6:40 PM
ardumont updated this revision to Diff 13991.Sep 16 2020, 6:50 PM

Rework commit message

swh-public-ci added a comment.Sep 16 2020, 6:54 PM

Build is green

Patch application report for D3957 (id=13991)

Could not rebase; Attempt merge onto f41a5a6c06...

Updating f41a5a6c..e5eacbbf
Fast-forward
 requirements-swh.txt                           |  2 +-
 swh/deposit/api/private/deposit_read.py        | 24 ----------
 swh/deposit/client.py                          | 17 ++-----
 swh/deposit/config.py                          | 47 ++++++++-----------
 swh/deposit/loader/checker.py                  | 26 ++++-------
 swh/deposit/tests/api/test_deposit_schedule.py | 18 +++-----
 swh/deposit/tests/conftest.py                  | 64 +++++++++++++-------------
 swh/deposit/tests/loader/conftest.py           | 37 ++++-----------
 swh/deposit/tests/loader/test_checker.py       |  8 ++--
 swh/deposit/tests/loader/test_tasks.py         | 12 ++---
 10 files changed, 91 insertions(+), 164 deletions(-)
Changes applied before test
commit e5eacbbf8b3a12def6edb41f3ca4be20e1ba52e6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 15 18:34:38 2020 +0200

    config: Clarify configuration setup path for the client code
    
    Related to T1532

commit 7f5241191352fcb170ced98bed745827cde63cbd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 15 10:51:37 2020 +0200

    config: Clarify the configuration setup path for the server part
    
    Mixin SWHConfig is deprecated.
    
    Related to T1532

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

Harbormaster completed remote builds in B15203: Diff 13991.Sep 16 2020, 6:54 PM
Closed by commit rDDEPe5eacbbf8b3a: config: Clarify configuration setup path for the client code (authored by ardumont). · Explain WhySep 17 2020, 9:22 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDDEPe5eacbbf8b3a: config: Clarify configuration setup path for the client code.
ardumont mentioned this in T1532: Cleanup deprecated configuration code in swh modules.Oct 5 2020, 12:59 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
deposit/
17 lines49%
Loading...
loader/
26 lines100%
Loading...
tests/
loader/
37 lines100%
Loading...
8 lines100%
Loading...
12 lines100%
Loading...

Diff 13991

View Options

swh/deposit/client.py

# Copyright (C) 2019-2021 The Software Heritage developers # Copyright (C) 2019-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version # License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
from datetime import datetime, timedelta, timezone from datetime import datetime, timedelta, timezone
from itertools import permutations
from hypothesis import given, settings, strategies from hypothesis import given, settings, strategies
import pytest import pytest
from swh.core.api.classes import stream_results from swh.core.api.classes import stream_results
NOW = datetime.now(tz=timezone.utc).isoformat()
NOW_MINUS_5_HOURS = (datetime.now(tz=timezone.utc) - timedelta(hours=5)).isoformat()
NOW_PLUS_5_HOURS = (datetime.now(tz=timezone.utc) + timedelta(hours=5)).isoformat()
VISIT_STATUSES = [
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW_MINUS_5_HOURS,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_2",
"last_eventful_visit_date": NOW_PLUS_5_HOURS,
},
]
class CommonSearchTest: class CommonSearchTest:
def test_origin_url_unique_word_prefix(self): def test_origin_url_unique_word_prefix(self):
origin_foobar_baz = {"url": "http://foobar.baz"} origin_foobar_baz = {"url": "http://foobar.baz"}
origin_barbaz_qux = {"url": "http://barbaz.qux"} origin_barbaz_qux = {"url": "http://barbaz.qux"}
origin_qux_quux = {"url": "http://qux.quux"} origin_qux_quux = {"url": "http://qux.quux"}
origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux] origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux]
▲ Show 20 Lines • Show All 231 Lines • ▼ Show 20 Lines def test_origin_last_visit_date_update_search(self):
_check_min_last_visit_date(now_minus_5_hours) # Works for < _check_min_last_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError): with pytest.raises(AssertionError):
_check_min_last_visit_date(now_plus_5_hours) # Fails for > _check_min_last_visit_date(now_plus_5_hours) # Fails for >
_update_last_visit_date(now_plus_5_hours) _update_last_visit_date(now_plus_5_hours)
_check_min_last_visit_date(now_plus_5_hours) # Works for = _check_min_last_visit_date(now_plus_5_hours) # Works for =
_check_min_last_visit_date(now) # Works for < _check_min_last_visit_date(now) # Works for <
vsellierUnsubmitted
Not Done
Inline Actions

A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order gives the expected result
Please have a look at the equivalent on the scheduler:
https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/tests/test_journal_client.py$418

vsellier: A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order…
def test_journal_client_origin_visit_status_permutation(self):
for visit_statuses in permutations(VISIT_STATUSES, len(VISIT_STATUSES)):
self.search.origin_update(visit_statuses)
self.search.flush()
origin_url = "http://foobar.baz"
actual_page = self.search.origin_search(
url_pattern=origin_url, min_last_eventful_visit_date=NOW_PLUS_5_HOURS,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
def test_origin_last_eventful_visit_date_update_search(self):
origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url}])
self.search.flush()
def _update_last_eventful_visit_date(snapshot_id, last_eventful_visit_date):
self.search.origin_update(
[
{
"url": origin_url,
"snapshot_id": snapshot_id,
"last_eventful_visit_date": last_eventful_visit_date,
}
]
vsellierUnsubmitted
Not Done
Inline Actions

Is there a reason to use permutation instead of a parameterized test?
With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

@vlorentz WDYT ?

vsellier: Is there a reason to use permutation instead of a parameterized test? With this implementation…
vlorentzUnsubmitted
Not Done
Inline Actions

I agree

vlorentz: I agree
KShivenduAuthorUnsubmitted
Done
Inline Actions

I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work-with-unittest-class-based-tests which says.

unittest.TestCase methods cannot directly receive fixture function arguments as implementing that is likely to inflict on the ability to run general unittest.TestCase test suites.

Is this stackoverflow answer wrong ?

KShivendu: I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work…
KShivenduAuthorUnsubmitted
Done
Inline Actions

With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

Ahh, I see. My bad.

KShivendu: >> With this implementation the index is not reset between 2 iterations so after the first loop…
vlorentzUnsubmitted
Not Done
Inline Actions

This is not a unittest.TestCase-based test; it's just a regular class.

vlorentz: This is not a `unittest.TestCase`-based test; it's just a regular class.
)
self.search.flush()
def _check_min_last_eventful_visit_date(min_last_eventful_visit_date):
actual_page = self.search.origin_search(
url_pattern=origin_url,
min_last_eventful_visit_date=min_last_eventful_visit_date,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
now = datetime.now(tz=timezone.utc).isoformat()
now_minus_5_hours = (
datetime.now(tz=timezone.utc) - timedelta(hours=5)
).isoformat()
now_plus_5_hours = (
datetime.now(tz=timezone.utc) + timedelta(hours=5)
).isoformat()
snapshot_1 = "SNAPSHOT_1"
snapshot_2 = "SNAPSHOT_2"
_update_last_eventful_visit_date(snapshot_1, now)
_check_min_last_eventful_visit_date(now) # Works for =
_check_min_last_eventful_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours) # Fails for >
_update_last_eventful_visit_date(
snapshot_1, now_plus_5_hours
) # Revisit(not eventful) same origin
_check_min_last_eventful_visit_date(
now
) # Should remain the same because recent visit wasn't eventful
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours)
_update_last_eventful_visit_date(
snapshot_2, now_plus_5_hours
) # Revisit(eventful) same origin
_check_min_last_eventful_visit_date(now_plus_5_hours) # Works for =
_check_min_last_eventful_visit_date(now) # Works for <
def test_origin_update_with_no_visit_types(self): def test_origin_update_with_no_visit_types(self):
""" """
Update an origin with visit types first then with no visit types, Update an origin with visit types first then with no visit types,
check origin can still be searched with visit types afterwards. check origin can still be searched with visit types afterwards.
""" """
origin_url = "http://foobar.baz" origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}]) self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}])
self.search.flush() self.search.flush()
▲ Show 20 Lines • Show All 427 Lines • Show Last 20 Lines
View Options

swh/deposit/loader/checker.py

# Copyright (C) 2019-2021 The Software Heritage developers # Copyright (C) 2019-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version # License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
from datetime import datetime, timedelta, timezone from datetime import datetime, timedelta, timezone
from itertools import permutations
from hypothesis import given, settings, strategies from hypothesis import given, settings, strategies
import pytest import pytest
from swh.core.api.classes import stream_results from swh.core.api.classes import stream_results
NOW = datetime.now(tz=timezone.utc).isoformat()
NOW_MINUS_5_HOURS = (datetime.now(tz=timezone.utc) - timedelta(hours=5)).isoformat()
NOW_PLUS_5_HOURS = (datetime.now(tz=timezone.utc) + timedelta(hours=5)).isoformat()
VISIT_STATUSES = [
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW_MINUS_5_HOURS,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_2",
"last_eventful_visit_date": NOW_PLUS_5_HOURS,
},
]
class CommonSearchTest: class CommonSearchTest:
def test_origin_url_unique_word_prefix(self): def test_origin_url_unique_word_prefix(self):
origin_foobar_baz = {"url": "http://foobar.baz"} origin_foobar_baz = {"url": "http://foobar.baz"}
origin_barbaz_qux = {"url": "http://barbaz.qux"} origin_barbaz_qux = {"url": "http://barbaz.qux"}
origin_qux_quux = {"url": "http://qux.quux"} origin_qux_quux = {"url": "http://qux.quux"}
origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux] origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux]
▲ Show 20 Lines • Show All 231 Lines • ▼ Show 20 Lines def test_origin_last_visit_date_update_search(self):
_check_min_last_visit_date(now_minus_5_hours) # Works for < _check_min_last_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError): with pytest.raises(AssertionError):
_check_min_last_visit_date(now_plus_5_hours) # Fails for > _check_min_last_visit_date(now_plus_5_hours) # Fails for >
_update_last_visit_date(now_plus_5_hours) _update_last_visit_date(now_plus_5_hours)
_check_min_last_visit_date(now_plus_5_hours) # Works for = _check_min_last_visit_date(now_plus_5_hours) # Works for =
_check_min_last_visit_date(now) # Works for < _check_min_last_visit_date(now) # Works for <
vsellierUnsubmitted
Not Done
Inline Actions

A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order gives the expected result
Please have a look at the equivalent on the scheduler:
https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/tests/test_journal_client.py$418

vsellier: A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order…
def test_journal_client_origin_visit_status_permutation(self):
for visit_statuses in permutations(VISIT_STATUSES, len(VISIT_STATUSES)):
self.search.origin_update(visit_statuses)
self.search.flush()
origin_url = "http://foobar.baz"
actual_page = self.search.origin_search(
url_pattern=origin_url, min_last_eventful_visit_date=NOW_PLUS_5_HOURS,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
def test_origin_last_eventful_visit_date_update_search(self):
origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url}])
self.search.flush()
def _update_last_eventful_visit_date(snapshot_id, last_eventful_visit_date):
self.search.origin_update(
[
{
"url": origin_url,
"snapshot_id": snapshot_id,
"last_eventful_visit_date": last_eventful_visit_date,
}
]
vsellierUnsubmitted
Not Done
Inline Actions

Is there a reason to use permutation instead of a parameterized test?
With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

@vlorentz WDYT ?

vsellier: Is there a reason to use permutation instead of a parameterized test? With this implementation…
vlorentzUnsubmitted
Not Done
Inline Actions

I agree

vlorentz: I agree
KShivenduAuthorUnsubmitted
Done
Inline Actions

I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work-with-unittest-class-based-tests which says.

unittest.TestCase methods cannot directly receive fixture function arguments as implementing that is likely to inflict on the ability to run general unittest.TestCase test suites.

Is this stackoverflow answer wrong ?

KShivendu: I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work…
KShivenduAuthorUnsubmitted
Done
Inline Actions

With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

Ahh, I see. My bad.

KShivendu: >> With this implementation the index is not reset between 2 iterations so after the first loop…
vlorentzUnsubmitted
Not Done
Inline Actions

This is not a unittest.TestCase-based test; it's just a regular class.

vlorentz: This is not a `unittest.TestCase`-based test; it's just a regular class.
)
self.search.flush()
def _check_min_last_eventful_visit_date(min_last_eventful_visit_date):
actual_page = self.search.origin_search(
url_pattern=origin_url,
min_last_eventful_visit_date=min_last_eventful_visit_date,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
now = datetime.now(tz=timezone.utc).isoformat()
now_minus_5_hours = (
datetime.now(tz=timezone.utc) - timedelta(hours=5)
).isoformat()
now_plus_5_hours = (
datetime.now(tz=timezone.utc) + timedelta(hours=5)
).isoformat()
snapshot_1 = "SNAPSHOT_1"
snapshot_2 = "SNAPSHOT_2"
_update_last_eventful_visit_date(snapshot_1, now)
_check_min_last_eventful_visit_date(now) # Works for =
_check_min_last_eventful_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours) # Fails for >
_update_last_eventful_visit_date(
snapshot_1, now_plus_5_hours
) # Revisit(not eventful) same origin
_check_min_last_eventful_visit_date(
now
) # Should remain the same because recent visit wasn't eventful
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours)
_update_last_eventful_visit_date(
snapshot_2, now_plus_5_hours
) # Revisit(eventful) same origin
_check_min_last_eventful_visit_date(now_plus_5_hours) # Works for =
_check_min_last_eventful_visit_date(now) # Works for <
def test_origin_update_with_no_visit_types(self): def test_origin_update_with_no_visit_types(self):
""" """
Update an origin with visit types first then with no visit types, Update an origin with visit types first then with no visit types,
check origin can still be searched with visit types afterwards. check origin can still be searched with visit types afterwards.
""" """
origin_url = "http://foobar.baz" origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}]) self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}])
self.search.flush() self.search.flush()
▲ Show 20 Lines • Show All 427 Lines • Show Last 20 Lines
View Options

swh/deposit/tests/loader/conftest.py

# Copyright (C) 2019-2021 The Software Heritage developers # Copyright (C) 2019-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version # License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
from datetime import datetime, timedelta, timezone from datetime import datetime, timedelta, timezone
from itertools import permutations
from hypothesis import given, settings, strategies from hypothesis import given, settings, strategies
import pytest import pytest
from swh.core.api.classes import stream_results from swh.core.api.classes import stream_results
NOW = datetime.now(tz=timezone.utc).isoformat()
NOW_MINUS_5_HOURS = (datetime.now(tz=timezone.utc) - timedelta(hours=5)).isoformat()
NOW_PLUS_5_HOURS = (datetime.now(tz=timezone.utc) + timedelta(hours=5)).isoformat()
VISIT_STATUSES = [
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW_MINUS_5_HOURS,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_2",
"last_eventful_visit_date": NOW_PLUS_5_HOURS,
},
]
class CommonSearchTest: class CommonSearchTest:
def test_origin_url_unique_word_prefix(self): def test_origin_url_unique_word_prefix(self):
origin_foobar_baz = {"url": "http://foobar.baz"} origin_foobar_baz = {"url": "http://foobar.baz"}
origin_barbaz_qux = {"url": "http://barbaz.qux"} origin_barbaz_qux = {"url": "http://barbaz.qux"}
origin_qux_quux = {"url": "http://qux.quux"} origin_qux_quux = {"url": "http://qux.quux"}
origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux] origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux]
▲ Show 20 Lines • Show All 231 Lines • ▼ Show 20 Lines def test_origin_last_visit_date_update_search(self):
_check_min_last_visit_date(now_minus_5_hours) # Works for < _check_min_last_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError): with pytest.raises(AssertionError):
_check_min_last_visit_date(now_plus_5_hours) # Fails for > _check_min_last_visit_date(now_plus_5_hours) # Fails for >
_update_last_visit_date(now_plus_5_hours) _update_last_visit_date(now_plus_5_hours)
_check_min_last_visit_date(now_plus_5_hours) # Works for = _check_min_last_visit_date(now_plus_5_hours) # Works for =
_check_min_last_visit_date(now) # Works for < _check_min_last_visit_date(now) # Works for <
vsellierUnsubmitted
Not Done
Inline Actions

A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order gives the expected result
Please have a look at the equivalent on the scheduler:
https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/tests/test_journal_client.py$418

vsellier: A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order…
def test_journal_client_origin_visit_status_permutation(self):
for visit_statuses in permutations(VISIT_STATUSES, len(VISIT_STATUSES)):
self.search.origin_update(visit_statuses)
self.search.flush()
origin_url = "http://foobar.baz"
actual_page = self.search.origin_search(
url_pattern=origin_url, min_last_eventful_visit_date=NOW_PLUS_5_HOURS,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
def test_origin_last_eventful_visit_date_update_search(self):
origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url}])
self.search.flush()
def _update_last_eventful_visit_date(snapshot_id, last_eventful_visit_date):
self.search.origin_update(
[
{
"url": origin_url,
"snapshot_id": snapshot_id,
"last_eventful_visit_date": last_eventful_visit_date,
}
]
vsellierUnsubmitted
Not Done
Inline Actions

Is there a reason to use permutation instead of a parameterized test?
With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

@vlorentz WDYT ?

vsellier: Is there a reason to use permutation instead of a parameterized test? With this implementation…
vlorentzUnsubmitted
Not Done
Inline Actions

I agree

vlorentz: I agree
KShivenduAuthorUnsubmitted
Done
Inline Actions

I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work-with-unittest-class-based-tests which says.

unittest.TestCase methods cannot directly receive fixture function arguments as implementing that is likely to inflict on the ability to run general unittest.TestCase test suites.

Is this stackoverflow answer wrong ?

KShivendu: I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work…
KShivenduAuthorUnsubmitted
Done
Inline Actions

With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

Ahh, I see. My bad.

KShivendu: >> With this implementation the index is not reset between 2 iterations so after the first loop…
vlorentzUnsubmitted
Not Done
Inline Actions

This is not a unittest.TestCase-based test; it's just a regular class.

vlorentz: This is not a `unittest.TestCase`-based test; it's just a regular class.
)
self.search.flush()
def _check_min_last_eventful_visit_date(min_last_eventful_visit_date):
actual_page = self.search.origin_search(
url_pattern=origin_url,
min_last_eventful_visit_date=min_last_eventful_visit_date,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
now = datetime.now(tz=timezone.utc).isoformat()
now_minus_5_hours = (
datetime.now(tz=timezone.utc) - timedelta(hours=5)
).isoformat()
now_plus_5_hours = (
datetime.now(tz=timezone.utc) + timedelta(hours=5)
).isoformat()
snapshot_1 = "SNAPSHOT_1"
snapshot_2 = "SNAPSHOT_2"
_update_last_eventful_visit_date(snapshot_1, now)
_check_min_last_eventful_visit_date(now) # Works for =
_check_min_last_eventful_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours) # Fails for >
_update_last_eventful_visit_date(
snapshot_1, now_plus_5_hours
) # Revisit(not eventful) same origin
_check_min_last_eventful_visit_date(
now
) # Should remain the same because recent visit wasn't eventful
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours)
_update_last_eventful_visit_date(
snapshot_2, now_plus_5_hours
) # Revisit(eventful) same origin
_check_min_last_eventful_visit_date(now_plus_5_hours) # Works for =
_check_min_last_eventful_visit_date(now) # Works for <
def test_origin_update_with_no_visit_types(self): def test_origin_update_with_no_visit_types(self):
""" """
Update an origin with visit types first then with no visit types, Update an origin with visit types first then with no visit types,
check origin can still be searched with visit types afterwards. check origin can still be searched with visit types afterwards.
""" """
origin_url = "http://foobar.baz" origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}]) self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}])
self.search.flush() self.search.flush()
▲ Show 20 Lines • Show All 427 Lines • Show Last 20 Lines
View Options

swh/deposit/tests/loader/test_checker.py

# Copyright (C) 2019-2021 The Software Heritage developers # Copyright (C) 2019-2021 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version # License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
from datetime import datetime, timedelta, timezone from datetime import datetime, timedelta, timezone
from itertools import permutations
from hypothesis import given, settings, strategies from hypothesis import given, settings, strategies
import pytest import pytest
from swh.core.api.classes import stream_results from swh.core.api.classes import stream_results
NOW = datetime.now(tz=timezone.utc).isoformat()
NOW_MINUS_5_HOURS = (datetime.now(tz=timezone.utc) - timedelta(hours=5)).isoformat()
NOW_PLUS_5_HOURS = (datetime.now(tz=timezone.utc) + timedelta(hours=5)).isoformat()
VISIT_STATUSES = [
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_1",
"last_eventful_visit_date": NOW_MINUS_5_HOURS,
},
{
"url": "http://foobar.baz",
"snapshot_id": "SNAPSHOT_2",
"last_eventful_visit_date": NOW_PLUS_5_HOURS,
},
]
class CommonSearchTest: class CommonSearchTest:
def test_origin_url_unique_word_prefix(self): def test_origin_url_unique_word_prefix(self):
origin_foobar_baz = {"url": "http://foobar.baz"} origin_foobar_baz = {"url": "http://foobar.baz"}
origin_barbaz_qux = {"url": "http://barbaz.qux"} origin_barbaz_qux = {"url": "http://barbaz.qux"}
origin_qux_quux = {"url": "http://qux.quux"} origin_qux_quux = {"url": "http://qux.quux"}
origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux] origins = [origin_foobar_baz, origin_barbaz_qux, origin_qux_quux]
▲ Show 20 Lines • Show All 231 Lines • ▼ Show 20 Lines def test_origin_last_visit_date_update_search(self):
_check_min_last_visit_date(now_minus_5_hours) # Works for < _check_min_last_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError): with pytest.raises(AssertionError):
_check_min_last_visit_date(now_plus_5_hours) # Fails for > _check_min_last_visit_date(now_plus_5_hours) # Fails for >
_update_last_visit_date(now_plus_5_hours) _update_last_visit_date(now_plus_5_hours)
_check_min_last_visit_date(now_plus_5_hours) # Works for = _check_min_last_visit_date(now_plus_5_hours) # Works for =
_check_min_last_visit_date(now) # Works for < _check_min_last_visit_date(now) # Works for <
vsellierUnsubmitted
Not Done
Inline Actions

A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order gives the expected result
Please have a look at the equivalent on the scheduler:
https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/tests/test_journal_client.py$418

vsellier: A parameterized test could be useful to ensure having visits (eventful/uneventful) in any order…
def test_journal_client_origin_visit_status_permutation(self):
for visit_statuses in permutations(VISIT_STATUSES, len(VISIT_STATUSES)):
self.search.origin_update(visit_statuses)
self.search.flush()
origin_url = "http://foobar.baz"
actual_page = self.search.origin_search(
url_pattern=origin_url, min_last_eventful_visit_date=NOW_PLUS_5_HOURS,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
def test_origin_last_eventful_visit_date_update_search(self):
origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url}])
self.search.flush()
def _update_last_eventful_visit_date(snapshot_id, last_eventful_visit_date):
self.search.origin_update(
[
{
"url": origin_url,
"snapshot_id": snapshot_id,
"last_eventful_visit_date": last_eventful_visit_date,
}
]
vsellierUnsubmitted
Not Done
Inline Actions

Is there a reason to use permutation instead of a parameterized test?
With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

@vlorentz WDYT ?

vsellier: Is there a reason to use permutation instead of a parameterized test? With this implementation…
vlorentzUnsubmitted
Not Done
Inline Actions

I agree

vlorentz: I agree
KShivenduAuthorUnsubmitted
Done
Inline Actions

I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work-with-unittest-class-based-tests which says.

unittest.TestCase methods cannot directly receive fixture function arguments as implementing that is likely to inflict on the ability to run general unittest.TestCase test suites.

Is this stackoverflow answer wrong ?

KShivendu: I read this https://stackoverflow.com/questions/18182251/does-pytest-parametrized-test-work…
KShivenduAuthorUnsubmitted
Done
Inline Actions

With this implementation the index is not reset between 2 iterations so after the first loop, the test will always be "adding a visit oldest or equal than what is in the index"

Ahh, I see. My bad.

KShivendu: >> With this implementation the index is not reset between 2 iterations so after the first loop…
vlorentzUnsubmitted
Not Done
Inline Actions

This is not a unittest.TestCase-based test; it's just a regular class.

vlorentz: This is not a `unittest.TestCase`-based test; it's just a regular class.
)
self.search.flush()
def _check_min_last_eventful_visit_date(min_last_eventful_visit_date):
actual_page = self.search.origin_search(
url_pattern=origin_url,
min_last_eventful_visit_date=min_last_eventful_visit_date,
)
assert actual_page.next_page_token is None
results = [r["url"] for r in actual_page.results]
expected_results = [origin_url]
assert sorted(results) == sorted(expected_results)
now = datetime.now(tz=timezone.utc).isoformat()
now_minus_5_hours = (
datetime.now(tz=timezone.utc) - timedelta(hours=5)
).isoformat()
now_plus_5_hours = (
datetime.now(tz=timezone.utc) + timedelta(hours=5)
).isoformat()
snapshot_1 = "SNAPSHOT_1"
snapshot_2 = "SNAPSHOT_2"
_update_last_eventful_visit_date(snapshot_1, now)
_check_min_last_eventful_visit_date(now) # Works for =
_check_min_last_eventful_visit_date(now_minus_5_hours) # Works for <
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours) # Fails for >
_update_last_eventful_visit_date(
snapshot_1, now_plus_5_hours
) # Revisit(not eventful) same origin
_check_min_last_eventful_visit_date(
now
) # Should remain the same because recent visit wasn't eventful
with pytest.raises(AssertionError):
_check_min_last_eventful_visit_date(now_plus_5_hours)
_update_last_eventful_visit_date(
snapshot_2, now_plus_5_hours
) # Revisit(eventful) same origin
_check_min_last_eventful_visit_date(now_plus_5_hours) # Works for =
_check_min_last_eventful_visit_date(now) # Works for <
def test_origin_update_with_no_visit_types(self): def test_origin_update_with_no_visit_types(self):
""" """
Update an origin with visit types first then with no visit types, Update an origin with visit types first then with no visit types,
check origin can still be searched with visit types afterwards. check origin can still be searched with visit types afterwards.
""" """
origin_url = "http://foobar.baz" origin_url = "http://foobar.baz"
self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}]) self.search.origin_update([{"url": origin_url, "visit_types": ["git"]}])
self.search.flush() self.search.flush()
▲ Show 20 Lines • Show All 427 Lines • Show Last 20 Lines
View Options

swh/deposit/tests/loader/test_tasks.py

Loading...
About · Developer information · Legal