Skip to content

Conversation

osmaczko
Copy link
Contributor

iterates: #6792

@status-im-auto
Copy link
Member

status-im-auto commented Sep 27, 2025

Jenkins Builds

Click to see older builds (6)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ ff630b7 #1 2025-09-27 08:55:00 ~3 min linux/status-go 📦zip
✔️ ff630b7 #1 2025-09-27 08:55:08 ~3 min macos/status-go 📦zip
✔️ ff630b7 #1 2025-09-27 08:57:16 ~5 min windows/status-go 📦zip
ff630b7 #1 2025-09-27 09:00:18 ~8 min linux/nwaku 📄log
✔️ ff630b7 #1 2025-09-27 09:02:02 ~10 min tests-rpc 📄log
✔️ ff630b7 #1 2025-09-27 09:18:16 ~26 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 9348690 #2 2025-09-27 09:45:05 ~3 min macos/status-go 📦zip
✔️ 9348690 #2 2025-09-27 09:45:26 ~3 min linux/status-go 📦zip
✔️ 9348690 #2 2025-09-27 09:47:13 ~5 min windows/status-go 📦zip
✔️ 9348690 #2 2025-09-27 09:51:44 ~9 min tests-rpc 📄log
✔️ 9348690 #2 2025-09-27 09:52:33 ~10 min linux/nwaku 📦zip
✔️ 9348690 #2 2025-09-27 10:07:44 ~25 min tests 📄log
✔️ 476d6a2 #3 2025-09-27 13:10:50 ~3 min macos/status-go 📦zip
✔️ 476d6a2 #3 2025-09-27 13:11:08 ~3 min linux/status-go 📦zip
✔️ 476d6a2 #3 2025-09-27 13:13:05 ~5 min windows/status-go 📦zip
✔️ 476d6a2 #3 2025-09-27 13:17:16 ~9 min tests-rpc 📄log
✔️ 476d6a2 #3 2025-09-27 13:18:26 ~10 min linux/nwaku 📦zip
✔️ 476d6a2 #3 2025-09-27 13:34:18 ~26 min tests 📄log

Copy link

codecov bot commented Sep 27, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.83%. Comparing base (a384854) to head (476d6a2).

Files with missing lines Patch % Lines
messaging/adapters/persistence.go 31.57% 11 Missing and 2 partials ⚠️
protocol/messaging_persistence.go 78.26% 7 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6962      +/-   ##
===========================================
+ Coverage    59.52%   59.83%   +0.31%     
===========================================
  Files          810      809       -1     
  Lines       119476   119458      -18     
===========================================
+ Hits         71121    71481     +360     
+ Misses       41018    40611     -407     
- Partials      7337     7366      +29     
Flag Coverage Δ
functional 32.97% <33.33%> (+1.01%) ⬆️
unit 55.51% <60.86%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
messaging/core.go 56.81% <100.00%> (+1.54%) ⬆️
messaging/waku/gowaku.go 65.66% <100.00%> (-0.32%) ⬇️
protocol/messaging_persistence.go 74.37% <78.26%> (+0.65%) ⬆️
messaging/adapters/persistence.go 58.06% <31.57%> (-41.94%) ⬇️

... and 52 files with indirect coverage changes

@osmaczko osmaczko force-pushed the refactor/waku-protected-topics-persistence branch from ff630b7 to 9348690 Compare September 27, 2025 09:41
@osmaczko osmaczko force-pushed the refactor/waku-protected-topics-persistence branch from 9348690 to 476d6a2 Compare September 27, 2025 13:07
Copy link
Member

@jrainville jrainville left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this shouldn't be in persistence subpackage, but in waku package directly 🤔

To be used like waku.ProtectedTopic.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps generate a stub with gomock?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a mock has a proper use case here. I don't want to verify behavior, but rather provide functional persistence for tests. It isn't a stub either. I'll rename it to persistence_in_memory.go.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@osmaczko indeed. Don't bother, it's not that important

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants