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

Fix diff_test and test code cleanup#1350

Open
marcsugiyama wants to merge 2 commits intomasterfrom
sugiyama/fix-diff_test
Open

Fix diff_test and test code cleanup#1350
marcsugiyama wants to merge 2 commits intomasterfrom
sugiyama/fix-diff_test

Conversation

@marcsugiyama
Copy link
Copy Markdown

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
Copy Markdown
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.

Comment thread test/blockchain_ct_utils.erl Outdated
Comment thread test/blockchain_ct_utils.erl Outdated
@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
Copy Markdown
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