fix merge for lightpush client tests

tests/waku_lightpush/test_client.nim

@@ -22,14 +22,22 @@ import
 suite "Waku Lightpush Client":
   var
     handlerFuture {.threadvar.}: Future[(PubsubTopic, WakuMessage)]
+    handlerFutureFailsLightpush {.threadvar.}: Future[(PubsubTopic, WakuMessage)]
+
     handler {.threadvar.}: PushMessageHandler
+    handlerFailsLightpush {.threadvar.}: PushMessageHandler
 
     serverSwitch {.threadvar.}: Switch
+    serverSwitchFailsLightpush {.threadvar.}: Switch
     clientSwitch {.threadvar.}: Switch
+
     server {.threadvar.}: WakuLightPush
+    serverFailsLightpush {.threadvar.}: WakuLightPush
     client {.threadvar.}: WakuLightPushClient
 
     serverRemotePeerInfo {.threadvar.}: RemotePeerInfo
+    serverRemotePeerInfoFailsLightpush {.threadvar.}: RemotePeerInfo
+
     clientPeerId {.threadvar.}: PeerId
     pubsubTopic {.threadvar.}: PubsubTopic
     contentTopic {.threadvar.}: ContentTopic
@@ -48,21 +56,41 @@ suite "Waku Lightpush Client":
       # return that we published the message to 1 peer.
       return ok(1)
 
+    # A Lightpush server that fails
+    handlerFutureFailsLightpush = newPushHandlerFuture()
+    handlerFailsLightpush = proc(
+        peer: PeerId, pubsubTopic: PubsubTopic, message: WakuMessage
+    ): Future[WakuLightPushResult] {.async.} =
+      handlerFutureFailsLightpush.complete((pubsubTopic, message))
+      return lighpushErrorResult(
+        SERVICE_NOT_AVAILABLE, "a bad test server failed to push a lightpush message"
+      )
+
     serverSwitch = newTestSwitch()
+    serverSwitchFailsLightpush = newTestSwitch()
     clientSwitch = newTestSwitch()
+
     server = await newTestWakuLightpushNode(serverSwitch, handler)
+    serverFailsLightpush =
+      await newTestWakuLightpushNode(serverSwitchFailsLightpush, handlerFailsLightpush)
     client = newTestWakuLightpushClient(clientSwitch)
 
-    await allFutures(serverSwitch.start(), clientSwitch.start())
+    await allFutures(
+      serverSwitch.start(), serverSwitchFailsLightpush.start(), clientSwitch.start()
+    )
 
     serverRemotePeerInfo = serverSwitch.peerInfo.toRemotePeerInfo()
+    serverRemotePeerInfoFailsLightpush =
+      serverSwitchFailsLightpush.peerInfo.toRemotePeerInfo()
     clientPeerId = clientSwitch.peerInfo.peerId
     pubsubTopic = DefaultPubsubTopic
     contentTopic = DefaultContentTopic
     message = fakeWakuMessage()
 
   asyncTeardown:
-    await allFutures(clientSwitch.stop(), serverSwitch.stop())
+    await allFutures(
+      clientSwitch.stop(), serverSwitch.stop(), serverSwitchFailsLightpush.stop()
+    )
 
   suite "Verification of PushRequest Payload":
     asyncTest "Valid Payload Types":
@@ -338,7 +366,6 @@ suite "Waku Lightpush Client":
       check not publishResponse.isOk()
       check publishResponse.error.code == LightpushStatusCode.NO_PEERS_TO_RELAY
 
-  #[
     asyncTest "Positive Publish To Any":
       # add a peer that supports the Lightpush protocol to the client's PeerManager
       client.peerManager.addPeer(serverRemotePeerInfo) # supports Lightpush
@@ -360,6 +387,8 @@ suite "Waku Lightpush Client":
       # Then the response is negative
       check not publishResponse.isOk()
 
+    #[
+    # TODO: adapt test for reputation-based peer selection after reputation logic is moved into PeerManager
     asyncTest "Peer Selection for Lighpush with Reputation":
       # add a peer that does not support the Lightpush protocol to the client's PeerManager
       client.peerManager.addPeer(serverRemotePeerInfoFailsLightpush)

waku/waku_lightpush/client.nim

@@ -97,19 +97,19 @@ proc publish*(
   )
   let publishedCount = ?await wl.sendPushRequest(pushRequest, peer)
 
-  # FIXME: adapt for Lightpush v3 error reporting
-  #[
-  if pushResult.isErr:
-    if wl.reputationManager.isSome:
-      wl.reputationManager.get().setReputation(peer.peerId, some(false))
-    return err(pushResult.error)
-  ]#
-  
   for obs in wl.publishObservers:
     obs.onMessagePublished(pubSubTopic.get(""), message)
 
+  # FIXME: where is negative result returned?
+  # we should check publish result for adjusting reputation
+  # but it's unclear where to check it, hence checking publishedCount
+  if publishedCount == 0:
+    if wl.reputationManager.isSome:
+      wl.reputationManager.get().setReputation(peer.peerId, some(false))
+
   return lightpushSuccessResult(publishedCount)
 
+# TODO: move selectPeerForLightPush logic into PeerManager
 proc selectPeerForLightPush*(
     wl: WakuLightPushClient
 ): Future[Result[RemotePeerInfo, string]] {.async, gcsafe.} =
@@ -121,7 +121,8 @@ proc selectPeerForLightPush*(
       return err("could not retrieve a peer supporting WakuLightPushCodec")
     if wl.reputationManager.isSome():
       let reputation = wl.reputationManager.get().getReputation(candidate.peerId)
-      info "Peer selected", peerId = candidate.peerId, reputation = $reputation, attempts = $attempts
+      info "Peer selected",
+        peerId = candidate.peerId, reputation = $reputation, attempts = $attempts
       if (reputation == some(false)):
         attempts += 1
         continue
@@ -147,14 +148,6 @@ proc publishToAny*(
   )
   let publishedCount = ?await wl.sendPushRequest(pushRequest, peer)
 
-  # FIXME
-  #[
-  let peer = wl.peerManager.selectPeer(WakuLightPushCodec).valueOr:
-    return err("could not retrieve a peer supporting WakuLightPushCodec")
-  let pushRequest = PushRequest(pubSubTopic: pubSubTopic, message: message)
-  ?await wl.sendPushRequest(pushRequest, peer)
-  ]#
-
   for obs in wl.publishObservers:
     obs.onMessagePublished(pubSubTopic, message)