Chore: store v3 edge test cases #76
Conversation
fbarbu15
left a comment
fbarbu15
left a comment
There was a problem hiding this comment.
LGTM, left minor comments
Please review the logs, I think they could be better
tests/store/test_api_flags.py
Outdated
| logger.debug(f"logger is {wrong_peer_addr}") | ||
| try: | ||
| self.check_published_message_is_stored(store_node=self.store_node1, peer_addr=wrong_peer_addr) | ||
| raise Exception("message restored with wrong peer address") |
There was a problem hiding this comment.
Also let's please capitalize logs, this is a general comment
tests/store/test_api_flags.py
Outdated
| def test_store_with_wrongPeerAddr(self): | ||
| self.publish_message() | ||
| wrong_peer_addr = self.multiaddr_list[0][1:] | ||
| logger.debug(f"logger is {wrong_peer_addr}") |
There was a problem hiding this comment.
Maybe a better log would be:
f"Running test with wrong_peer_addr: {wrong_peer_addr}"
tests/store/test_api_flags.py
Outdated
|
|
||
| def test_store_not_include_data(self): | ||
| message = self.create_message() | ||
| self.publish_message(message=message) |
There was a problem hiding this comment.
in this case where we don't use message in any other place I think it's better to use directly:
self.publish_message(message=self.create_message())
makes test more succint
| return None | ||
|
|
||
| def message_payload(self, index): | ||
| def message_content(self, index): |
There was a problem hiding this comment.
this method was used in other places, please either:
- create a new method and leave message_payload
- update any other places where message_payload was used to use message_content
tests/store/test_api_flags.py
Outdated
| logger.debug(f" Message restored with hash only is {store_response.messages} ") | ||
| assert "message" not in store_response.messages | ||
|
|
||
| def test_get_store_messages_with_different_pubsub_topics11(self): |
There was a problem hiding this comment.
test already exists here: https://github.com/waku-org/waku-interop-tests/blob/master/tests/store/test_topics.py#L44
tests/store/test_api_flags.py
Outdated
| logger.error(f"Topic {wrong_topic} is wrong ''n: {str(e)}") | ||
| assert e.args[0].find("messages': []") != -1, "Message shall not be stored for wrong topic" | ||
|
|
||
| def test_get_store_messages_with_content_topic(self): |
There was a problem hiding this comment.
we already cover positive scenarios ex: https://github.com/waku-org/waku-interop-tests/blob/master/tests/store/test_topics.py#L17
tests/store/test_api_flags.py
Outdated
| def test_get_store_messages_with_content_topic(self): | ||
| # positive scenario | ||
| content_topic = "/myapp/1/latest/protoo" | ||
| message = {"payload": to_base64(self.test_payload), "" "contentTopic": content_topic, "timestamp": int(time() * 1e9)} |
There was a problem hiding this comment.
we can use create_message ex
message = self.create_message(contentTopic=content_topic)
|
@AYAHASSAN287 can you please update the PR name and description, saying what was added? |
|
@AYAHASSAN287 I also just noticed that you didn't run the tests in the CI.
|
|
@AYAHASSAN287 not sure if you are aware but those tests are running in CI under 2 workflows:
Did you tested your new tests with both flows? If they pass you don't need to do anything. but if they fail on nim -> go, they need either adjusting or skipping |
fbarbu15
changed the title
PR Details
Issues reported: