Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

Fix diff_test and test code cleanup #1350

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

marcsugiyama
Copy link

diff_test calls blockchain_gossip_handler:add_block with the Swarm instead of the SwarmTID which results in a badarg from ets. This PR changes the call to pass the table id instead, fixing the test error.

This PR also includes some code cleanup as suggested by @xandkar .

@marcsugiyama marcsugiyama requested review from evanmcc and xandkar May 17, 2022 18:47
Copy link
Contributor

@xandkar xandkar left a comment

Choose a reason for hiding this comment

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

Looks good.

Left some minor comments. Also, in general, I still think that just calling digraph_utils:components/1 on the topology would be much easier to understand.

test/blockchain_ct_utils.erl Outdated Show resolved Hide resolved
test/blockchain_ct_utils.erl Outdated Show resolved Hide resolved
@marcsugiyama marcsugiyama force-pushed the sugiyama/fix-diff_test branch from d723ec9 to b32b2e0 Compare May 19, 2022 18:35
% NetworkMap - #{ConnectedToNode => [ConnectedFromNode]}
find_connected_node_pair({_, Node, Iter}, AddrToNodeMap, NetworkMap) ->
GossipPeerNodes =
[ConnectedNode | _] =
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're only picking one, maybe no need to addr_to_node all of them, but just lookup the picked one?

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

Successfully merging this pull request may close these issues.

2 participants