Page MenuHomeSoftware Heritage

Add tests for Sentry reporting + fix inconsistent capturing
ClosedPublic

Authored by vlorentz on Aug 8 2022, 1:55 PM.

Diff Detail

Repository
rDCIDX Metadata indexer
Branch
sentry
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 30678
Build 47968: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 47967: arc lint + arc unit

Event Timeline

vlorentz retitled this revision from Add tests for Sentry reporting + add inconsistent capturing to Add tests for Sentry reporting + fix inconsistent capturing.Aug 8 2022, 2:01 PM

Build is green

Patch application report for D8214 (id=29613)

Could not rebase; Attempt merge onto 98153fb7ed...

Updating 98153fb..f6547d2
Fast-forward
 swh/indexer/indexer.py                    |  5 ++
 swh/indexer/metadata.py                   |  9 +---
 swh/indexer/tests/conftest.py             | 54 +++++++++++++++++++++
 swh/indexer/tests/test_indexer.py         | 79 ++++++++++++++++++++++++++-----
 swh/indexer/tests/test_origin_metadata.py | 49 ++++++++++++++++++-
 5 files changed, 176 insertions(+), 20 deletions(-)
Changes applied before test
commit f6547d2860338a3c54599d87f3dd159521bd40d7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Aug 8 13:49:33 2022 +0200

    Add tests for Sentry reporting + add inconsistent capturing

commit 5975d32319ef71abe1c91192b9a1adde537cc783
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Aug 8 12:03:06 2022 +0200

    Fix support of old RawExtrinsicMetadata objects with no id

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

Build was aborted

Patch application report for D8214 (id=29615)

Could not rebase; Attempt merge onto 98153fb7ed...

Updating 98153fb..b6b437e
Fast-forward
 swh/indexer/indexer.py                    |  5 ++
 swh/indexer/metadata.py                   |  9 +---
 swh/indexer/tests/conftest.py             | 54 +++++++++++++++++++++
 swh/indexer/tests/test_indexer.py         | 79 ++++++++++++++++++++++++++-----
 swh/indexer/tests/test_origin_metadata.py | 49 ++++++++++++++++++-
 5 files changed, 176 insertions(+), 20 deletions(-)
Changes applied before test
commit b6b437e8ca1f5936c5ffcf1663a2d89aff3e0b8d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Aug 8 13:49:33 2022 +0200

    Add tests for Sentry reporting + fix inconsistent capturing

commit 5975d32319ef71abe1c91192b9a1adde537cc783
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Aug 8 12:03:06 2022 +0200

    Fix support of old RawExtrinsicMetadata objects with no id

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/405/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tests-on-diff/405/console

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 8 2022, 2:14 PM
Harbormaster failed remote builds in B30680: Diff 29615!

I think you can simplify the new sentry fixtures to save some function calls in the tests, they are still passing with the diff below:

diff --git a/swh/indexer/tests/conftest.py b/swh/indexer/tests/conftest.py
index 048e8be..ef8e5c0 100644
--- a/swh/indexer/tests/conftest.py
+++ b/swh/indexer/tests/conftest.py
@@ -160,27 +160,26 @@ def sentry_init():
 
 
 @pytest.fixture
-def sentry_capture_events(monkeypatch):
+def sentry_events(monkeypatch, sentry_init):
     # Copied from
     # https://github.com/getsentry/sentry-python/blob/1.5.9/tests/conftest.py#L195-L217
-    def inner():
-        events = []
-        test_client = sentry_sdk.Hub.current.client
-        old_capture_event = test_client.transport.capture_event
-        old_capture_envelope = test_client.transport.capture_envelope
-
-        def append_event(event):
-            events.append(event)
-            return old_capture_event(event)
-
-        def append_envelope(envelope):
-            for item in envelope:
-                if item.headers.get("type") in ("event", "transaction"):
-                    test_client.transport.capture_event(item.payload.json)
-            return old_capture_envelope(envelope)
-
-        monkeypatch.setattr(test_client.transport, "capture_event", append_event)
-        monkeypatch.setattr(test_client.transport, "capture_envelope", append_envelope)
-        return events
-
-    return inner
+
+    sentry_init()
+    events = []
+    test_client = sentry_sdk.Hub.current.client
+    old_capture_event = test_client.transport.capture_event
+    old_capture_envelope = test_client.transport.capture_envelope
+
+    def append_event(event):
+        events.append(event)
+        return old_capture_event(event)
+
+    def append_envelope(envelope):
+        for item in envelope:
+            if item.headers.get("type") in ("event", "transaction"):
+                test_client.transport.capture_event(item.payload.json)
+        return old_capture_envelope(envelope)
+
+    monkeypatch.setattr(test_client.transport, "capture_event", append_event)
+    monkeypatch.setattr(test_client.transport, "capture_envelope", append_envelope)
+    return events
diff --git a/swh/indexer/tests/test_indexer.py b/swh/indexer/tests/test_indexer.py
index 9518f50..fd4d3c2 100644
--- a/swh/indexer/tests/test_indexer.py
+++ b/swh/indexer/tests/test_indexer.py
@@ -79,15 +79,12 @@ def check_sentry(sentry_events):
     assert "'_TestException'" in str(sentry_event)
 
 
-def test_content_indexer_catch_exceptions(sentry_init, sentry_capture_events):
+def test_content_indexer_catch_exceptions(sentry_events):
     indexer = CrashingContentIndexer(config=BASE_TEST_CONFIG)
     indexer.objstorage = Mock()
     indexer.objstorage.get.return_value = b"content"
     indexer.objstorage.get_batch.return_value = [b"content"]
 
-    sentry_init()
-    sentry_events = sentry_capture_events()
-
     sha1 = b"\x12" * 20
 
     # As task, catching exceptions
@@ -115,14 +112,11 @@ def test_content_indexer_catch_exceptions(sentry_init, sentry_capture_events):
     assert sentry_events == []
 
 
-def test_directory_indexer_catch_exceptions(sentry_init, sentry_capture_events):
+def test_directory_indexer_catch_exceptions(sentry_events):
     indexer = CrashingDirectoryIndexer(config=BASE_TEST_CONFIG)
     indexer.storage = Mock()
     indexer.storage.directory_get.return_value = [DIRECTORY2]
 
-    sentry_init()
-    sentry_events = sentry_capture_events()
-
     sha1 = DIRECTORY2.id
 
     # As task, catching exceptions
@@ -148,12 +142,9 @@ def test_directory_indexer_catch_exceptions(sentry_init, sentry_capture_events):
     assert sentry_events == []
 
 
-def test_origin_indexer_catch_exceptions(sentry_init, sentry_capture_events):
+def test_origin_indexer_catch_exceptions(sentry_events):
     indexer = CrashingOriginIndexer(config=BASE_TEST_CONFIG)
 
-    sentry_init()
-    sentry_events = sentry_capture_events()
-
     origin_url = "http://example.org"
 
     # As task, catching exceptions
diff --git a/swh/indexer/tests/test_origin_metadata.py b/swh/indexer/tests/test_origin_metadata.py
index 7c07d04..17ee955 100644
--- a/swh/indexer/tests/test_origin_metadata.py
+++ b/swh/indexer/tests/test_origin_metadata.py
@@ -264,16 +264,12 @@ def test_origin_metadata_indexer_directory_error(
     idx_storage: IndexerStorageInterface,
     storage: StorageInterface,
     obj_storage,
-    sentry_init,
-    sentry_capture_events,
+    sentry_events,
 ) -> None:
 
     indexer = OriginMetadataIndexer(config=swh_indexer_config)
     origin = "https://github.com/librariesio/yarn-parser"
 
-    sentry_init()
-    sentry_events = sentry_capture_events()
-
     with patch(
         "swh.indexer.metadata.DirectoryMetadataIndexer"
         ".translate_directory_intrinsic_metadata",
@@ -299,16 +295,12 @@ def test_origin_metadata_indexer_content_exception(
     idx_storage: IndexerStorageInterface,
     storage: StorageInterface,
     obj_storage,
-    sentry_init,
-    sentry_capture_events,
+    sentry_events,
 ) -> None:
 
     indexer = OriginMetadataIndexer(config=swh_indexer_config)
     origin = "https://github.com/librariesio/yarn-parser"
 
-    sentry_init()
-    sentry_events = sentry_capture_events()
-
     class TestException(Exception):
         pass

Build is green

Patch application report for D8214 (id=29615)

Rebasing onto 5975d32319...

Current branch diff-target is up to date.
Changes applied before test
commit b6b437e8ca1f5936c5ffcf1663a2d89aff3e0b8d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Aug 8 13:49:33 2022 +0200

    Add tests for Sentry reporting + fix inconsistent capturing

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

Build is green

Patch application report for D8214 (id=29624)

Rebasing onto 5975d32319...

Current branch diff-target is up to date.
Changes applied before test
commit 50a42eb3e89d6700927b20a4bcf9c9747fa8c010
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Aug 8 13:49:33 2022 +0200

    Add tests for Sentry reporting + fix inconsistent capturing

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

This revision is now accepted and ready to land.Aug 8 2022, 3:13 PM