Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase libp2ps connections code coverage to 100% #1497

Merged
merged 7 commits into from
Jul 14, 2020

Conversation

lrahmani
Copy link
Collaborator

@lrahmani lrahmani commented Jul 14, 2020

Proposed changes

  • Increase p2p_libp2p connection code coverage to 100%: with 29 out of 351 statements excluded
  • Increase p2p_libp2p_client connection code coverage to 100%: with 18 out pf 145 statements excluded

Fixes #1468, #1162

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2020

Codecov Report

Merging #1497 into develop will increase coverage by 0.34%.
The diff coverage is 71.42%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1497      +/-   ##
===========================================
+ Coverage    89.92%   90.27%   +0.34%     
===========================================
  Files          243      243              
  Lines        17239    17209      -30     
===========================================
+ Hits         15503    15536      +33     
+ Misses        1736     1673      -63     
Flag Coverage Δ
#unittests 90.27% <71.42%> (+0.34%) ⬆️
Impacted Files Coverage Δ
...kages/fetchai/connections/p2p_libp2p/connection.py 97.81% <66.66%> (+12.22%) ⬆️
...etchai/connections/p2p_libp2p_client/connection.py 92.91% <75.00%> (+9.34%) ⬆️
aea/connections/stub/connection.py 100.00% <0.00%> (ø)
packages/fetchai/connections/soef/connection.py 100.00% <0.00%> (ø)
packages/fetchai/connections/local/connection.py 100.00% <0.00%> (ø)
aea/helpers/async_friendly_queue.py 94.11% <0.00%> (+0.17%) ⬆️
aea/helpers/base.py 90.00% <0.00%> (+0.66%) ⬆️
aea/launcher.py 98.03% <0.00%> (+4.90%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34e4ae1...15b7ecf. Read the comment docs.

assert self._in_queue is not None, "Input queue not initialized."
self._in_queue.put_nowait(data)
if data is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this was moved here; data is already used in line 706

Copy link
Contributor

Choose a reason for hiding this comment

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

Now you might be putting None on the queue

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, exactly.
The _receive_from_node task will exit and the multiplexer will receive None on its scheduled receive

Comment on lines -296 to -298
which = shutil.which("go")
if which is None:
raise Exception("Libp2p Go should me installed")
Copy link
Contributor

Choose a reason for hiding this comment

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

why has this been removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because connection already does the check (using _check_go_installed())

Copy link
Contributor

@DavidMinarsch DavidMinarsch left a comment

Choose a reason for hiding this comment

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

LGTM

@DavidMinarsch DavidMinarsch merged commit a5f915b into develop Jul 14, 2020
Karrenbelt pushed a commit to valory-xyz/open-aea that referenced this pull request Feb 11, 2022
…overage

Increase libp2ps connections code coverage to 100%
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants