Qa wrappers #19

AYAHASSAN287 · 2025-01-21T19:44:42Z

Pull Request Description

This pull request introduces the testlibs framework for Go-Waku Bindings, which provides wrappers, utilities, and test files to simplify the development and testing process of Waku Node functionalities. The testlibs framework focuses on enhancing developer productivity and streamlining common tasks through the use of high-level wrapper APIs.

Advantages of the Wrappers APIs

The wrapper APIs provide several key advantages over directly using internal APIs:

Simplified Inputs:
- Wrapper APIs abstract away low-level details, making them easier to use. For example:
  - Wrappers_ConnectPeer takes a WakuNode instance as input rather than requiring a peerID, internally constructing the peerID from the provided node.
  - This reduces boilerplate code, such as manually fetching peerID or address details before connecting.
Error Handling and Validation:
- Wrappers include built-in validation to ensure inputs are valid before invoking internal APIs. For example:
  - Wrappers_GetConnectedPeers checks if the WakuNode is nil and returns an appropriate error message, preventing runtime crashes.
Consistent Logging:
- Each wrapper API includes debug-level logging to trace execution flow and identify issues. This is particularly helpful during test execution and debugging.
Streamlined Node Lifecycle Management:
- The wrappers combine multiple operations into intuitive methods, such as:
  - Wrappers_StopAndDestroy: Combines stopping and destroying a Waku node into a single operation.
- This reduces the need to call multiple methods manually and minimizes cleanup errors.
Enhanced Developer Experience:
- The high-level wrappers are designed to match common use cases in testing, reducing the cognitive load on developers:
  - Relay protocol management is simplified through methods like Wrappers_RelaySubscribe and Wrappers_RelayUnsubscribe, which handle both subscription and verification.
Dynamic Configuration:
- Wrapper APIs allow seamless handling of dynamic configurations, such as generating unique ports (GenerateUniquePort) for nodes during testing.

Key Components

1. Wrappers

The wrapper files define abstractions around the WakuNode to streamline node management and interaction during testing. Key functionalities include:

Node Lifecycle Management:
- Wrappers_StartWakuNode: Starts a Waku node with a custom or default configuration.
- Wrappers_Stop: Stops the Waku node gracefully.
- Wrappers_Destroy: Cleans up and destroys the Waku node.
- Wrappers_StopAndDestroy: Combines stop and destroy operations for efficient cleanup.
Peer Management:
- Wrappers_ConnectPeer: Simplifies connecting nodes by taking a WakuNode instance instead of a peerID, constructing the peerID and connection details internally.
- Wrappers_DisconnectPeer: Disconnects a Waku node from a target peer with validation and error handling.
- Wrappers_GetConnectedPeers: Retrieves a list of peers connected to the Waku node.
- Wrappers_GetNumConnectedRelayPeers: Returns the number of relay peers connected for a specific pubsub topic.
Relay Subscription:
- Wrappers_RelaySubscribe: Subscribes the Waku node to a given pubsub topic, ensuring subscription and verifying peer connections.
- Wrappers_RelayUnsubscribe: Unsubscribes from a specific pubsub topic and validates the unsubscription process.

2. Utilities

The utilities package provides helper functions and constants to facilitate testing. Key highlights include:

Default Configuration:
- DefaultWakuConfig: Provides a baseline Waku node configuration with sensible defaults for testing purposes.
Logging Support:
- Centralized logging via zap for debugging during tests.
- Debug-level messages are used extensively to trace the flow and identify issues.
Port Management:
- GenerateUniquePort: Dynamically allocates unique ports for Waku nodes during tests to avoid conflicts.
Timeout and Error Handling:
- Constants for peer connection timeouts.
- Enhanced error messaging for debugging failures in Waku node operations.

3. Test Files

Relay Tests (`relay_test.go`)

Highlights:
- Tests the behavior of the relay protocol.
- Example: TestRelaySubscribeToDefaultTopic validates subscription to the default pubsub topic and ensures connected relay peers increase.

Peer Connection Tests (`Peers_connection_test.go`)

Highlights:
- Simplifies peer connectivity testing using the wrappers.
- Example: TestConnectMultipleNodesToSingleNode verifies multiple nodes can connect to a single node efficiently.

Basic Node Tests (`Nodes_basic_test.go`)

Highlights:
- Validates fundamental node lifecycle management using wrapper APIs.
- Example: TestBasicWakuNodes covers node creation, startup, and cleanup using Wrappers_StartWakuNode and Wrappers_StopAndDestroy.

fbarbu15 · 2025-01-28T09:39:37Z

testlibs/src/Relay_wrapper.go

not sure what's the best practice in golang but I think filenames should be lowercase.
Anyhow we should have consistency

fbarbu15 · 2025-01-28T09:41:09Z

testlibs/README.md

@@ -0,0 +1,23 @@
+# Testlibs for Go-Waku Bindings


Maybe you should add the Key Components section from PR description here as well, what do you think?

right. since the Key Components contain more details it's better to include it in the README file too

Done

fbarbu15 · 2025-01-28T09:44:09Z

testlibs/src/testing-wrappers.go

+	return nil
+}
+
+func (wrapper *WakuNodeWrapper) Wrappers_StopAndDestroy() error {


do we need this function since we already have both Wrappers_Stop and Wrappers_Destroy ?

yes because we may need to stop the node and resume or start it again without destroying it

fbarbu15 · 2025-01-28T09:46:09Z

testlibs/src/testing-wrappers.go

+func (wrapper *WakuNodeWrapper) Wrappers_GetNumConnectedRelayPeers(optPubsubTopic ...string) (int, error) {
+	utilities.Debug("Wrappers_GetNumConnectedRelayPeers called")
+
+	if wrapper.WakuNode == nil {


you do the check quite a lot, could it be refactored into a common function?

yes for better readability
I will do that

fbarbu15 · 2025-01-28T09:48:29Z

testlibs/waku-go-tests/Nodes_basic_test.go

+	time.Sleep(2 * time.Second)
+	utilities.Debug("Stopping the first WakuNodeWrapper")
+
+	err = node1.Wrappers_StopAndDestroy()


hmm, can we have this as a cleanup/teardown construct? so it gets executed even if the tests fails at an assertion

fbarbu15 · 2025-01-28T09:50:10Z

testlibs/waku-go-tests/Peers_connection_test.go

+	time.Sleep(3 * time.Second)
+
+	// Disconnect Node A from Node B
+	err = nodeA.Wrappers_DisconnectPeer(nodeB)


in those tests we are no stopping the nodes?

at line 22
defer nodeA.Wrappers_StopAndDestroy()
it' sthe teardown way in GO language

fbarbu15 · 2025-01-28T09:56:58Z

testlibs/waku-go-tests/Nodes_basic_test.go

+	utilities.Debug("Successfully created the first WakuNodeWrapper")
+
+	utilities.Debug("Starting the second WakuNodeWrapper")
+	node2, err := testlibs.Wrappers_StartWakuNode(&nodeCfg2, logger.Named("node2"))


do we need 2 nodes here? because you don't connect them
I guess one node is enough

Correct. node2 removed

fbarbu15 · 2025-01-28T10:00:40Z

testlibs/waku-go-tests/relay_test.go

+	"go.uber.org/zap"
+)
+
+func TestRelaySubscribeToDefaultTopic(t *testing.T) {


we should also have a test that runs and connects 2 nodes, sends message from node1 and checks it was received on node2

Was going to do that but currently relay tests failing for internal issue so I'm waiting for the PR to be merged by Gabriel then I will add additional steps

Yes! The functionality to receive messages is broken in master but we have PRs in place that fix it. We should have them merged soon, and I will update Aya once we merge them :)

fbarbu15

Great work! LGTM, thanks

gabrielmer

Wooow, amazing work! 🤩 Thanks so much!

Added just a few comments, let me know if something doesn't make sense!

gabrielmer · 2025-01-29T10:59:41Z

testlibs/src/main-wrappers.go

+	nodeCfg.Discv5UdpPort = utilities.GenerateUniquePort()
+	nodeCfg.TcpPort = utilities.GenerateUniquePort()
+
+	utilities.Debug("Create node with default config")


maybe this log is misleading? because if customCfg is passed, then it's not being created with the default config

gabrielmer · 2025-01-29T11:02:45Z

testlibs/src/main-wrappers.go

+	utilities.Debug("Create node with default config")
+	node, err := waku.NewWakuNode(&nodeCfg, logger)
+	if err != nil {
+		utilities.Error("Can't create node")


should we add the error in the log? something like

utilities.Error("Can't create node", zap.Error(err))

gabrielmer · 2025-01-29T11:13:55Z

testlibs/src/main-wrappers.go

+}
+
+// Stops the WakuNode.
+func (node *WakuNodeWrapper) Wrappers_Stop() error {


As this is a method of WakuNodeWrapper, maybe we can name it Stop() instead of Wrappers_Stop()?

Same for all the methods of WakuNodeWrapper, the Wrappers prefix might be redundant

gabrielmer · 2025-01-29T11:23:35Z

testlibs/src/relay_wrapper.go

+	numRelayPeers, err := wrapper.Wrappers_GetNumConnectedRelayPeers(pubsubTopic)
+	if err != nil || numRelayPeers == 0 {
+		utilities.Error("Subscription verification failed: no connected relay peers found", zap.Error(err))
+		return errors.New("subscription verification failed: no connected relay peers")


Not sure if verifying the number of peers in a pubsub topic is the way to verify that we correctly subscribed to it - we could be subscribed but without peers in the topic.

If we need a way to verify it, we could either add a function in waku-go-bindings that returns the subscribed topics, or we can probably deduce it from the ENR

gabrielmer · 2025-01-29T11:28:47Z

testlibs/waku-go-tests/relay_test.go

+	"go.uber.org/zap"
+)
+
+func TestRelaySubscribeToDefaultTopic(t *testing.T) {


Yes! The functionality to receive messages is broken in master but we have PRs in place that fix it. We should have them merged soon, and I will update Aya once we merge them :)

AYAHASSAN287 and others added 6 commits January 21, 2025 13:26

Adding wrappers and new test

80670e5

Add basic test new wrappers

8ccbabd

Adding more wrappers and test to connect and disconnect peers

18840fa

Adding peers connection tests & relay tests

8a3f6cf

reset nwaku_test.go to master

74bb730

Create README.md

b166398

AYAHASSAN287 requested a review from fbarbu15 January 28, 2025 09:08

fbarbu15 suggested changes Jan 28, 2025

View reviewed changes

AYAHASSAN287 and others added 4 commits January 28, 2025 12:21

Update README.md

3a6cce2

Fix review points

3223548

Merge remote-tracking branch 'origin/QA_Wrappers' into QA_Wrappers

d973e45

Fix part2 review points

b81b437

AYAHASSAN287 requested a review from fbarbu15 January 28, 2025 13:11

fbarbu15 approved these changes Jan 28, 2025

View reviewed changes

AYAHASSAN287 requested a review from gabrielmer January 29, 2025 10:25

gabrielmer requested a review from Ivansete-status January 29, 2025 11:30

gabrielmer reviewed Jan 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Qa wrappers #19

Qa wrappers #19

AYAHASSAN287 commented Jan 21, 2025 •

edited

Loading

fbarbu15 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

fbarbu15 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

fbarbu15 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

fbarbu15 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

fbarbu15 Jan 28, 2025

fbarbu15 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

fbarbu15 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

fbarbu15 Jan 28, 2025

AYAHASSAN287 Jan 28, 2025

gabrielmer Jan 29, 2025

fbarbu15 left a comment

gabrielmer left a comment

gabrielmer Jan 29, 2025

gabrielmer Jan 29, 2025

gabrielmer Jan 29, 2025

gabrielmer Jan 29, 2025

gabrielmer Jan 29, 2025

Qa wrappers #19

Are you sure you want to change the base?

Qa wrappers #19

Conversation

AYAHASSAN287 commented Jan 21, 2025 • edited Loading

Pull Request Description

Advantages of the Wrappers APIs

Key Components

1. Wrappers

2. Utilities

3. Test Files

Relay Tests (relay_test.go)

Peer Connection Tests (Peers_connection_test.go)

Basic Node Tests (Nodes_basic_test.go)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbarbu15 left a comment

Choose a reason for hiding this comment

gabrielmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AYAHASSAN287 commented Jan 21, 2025 •

edited

Loading

Relay Tests (`relay_test.go`)

Peer Connection Tests (`Peers_connection_test.go`)

Basic Node Tests (`Nodes_basic_test.go`)