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

Fix errors during unittest on Mono #236

Merged
merged 4 commits into from May 13, 2019

Conversation

Projects
None yet
3 participants
@longfin
Copy link
Member

commented May 11, 2019

This PR aids some unittest errors on Mono runtime.

  • Made Swarm.StopAsync() closes internal resources(e.g. _broadcastQueue) to avoid too many open files error.
    • Also, SwarmTest now cleans NetMQ resources at the end of each test case.
  • Bump .netframework version of Libplanet.Tests used by Mono to avoid missing method exception.

I omitted changelog because there is no difference from previous version.

@longfin longfin force-pushed the longfin:bugfix/swarm-closing branch from 92e975d to db28fb7 May 11, 2019

@longfin longfin marked this pull request as ready for review May 11, 2019

@codecov

This comment has been minimized.

Copy link

commented May 11, 2019

Codecov Report

Merging #236 into master will decrease coverage by 0.23%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage   83.69%   83.45%   -0.24%     
==========================================
  Files          75       75              
  Lines        3496     3494       -2     
==========================================
- Hits         2926     2916      -10     
- Misses        570      578       +8
Impacted Files Coverage Δ
Libplanet/Net/Swarm.cs 78.88% <100%> (-0.92%) ⬇️

@longfin longfin force-pushed the longfin:bugfix/swarm-closing branch 3 times, most recently from 82fbb9b to 51a8772 May 11, 2019

@earlbread earlbread self-requested a review May 11, 2019

@longfin longfin force-pushed the longfin:bugfix/swarm-closing branch 2 times, most recently from ed06553 to c446d3e May 11, 2019

@longfin longfin force-pushed the longfin:bugfix/swarm-closing branch 2 times, most recently from 6221e92 to 4474e0e May 12, 2019

@longfin longfin force-pushed the longfin:bugfix/swarm-closing branch from 4474e0e to b636543 May 12, 2019

@longfin

This comment has been minimized.

Copy link
Member Author

commented May 13, 2019

I've add some commit.

  • In macOS + .NET Core, NetMQConfig.Cleanup() seems to be hang. so added condition as workaround.
  • Remove StopAsync() in Swarm.StartAsync().

@longfin longfin requested a review from dahlia May 13, 2019

@@ -85,6 +85,7 @@ To be released.
- `BlockSet<T>[int]` changed so as not to validate a block. [[#231]]
- Improved read performance of `Block<T>.Hash` and `Transaction<T>.Id`. [[#228],
[#234]]
- `Swarm.StartAsync()` doesn't call `Swarm.StopAsync()` anymore.

This comment has been minimized.

Copy link
@dahlia

dahlia May 13, 2019

Member

So users need some action due to this change?

This comment has been minimized.

Copy link
@longfin

longfin May 13, 2019

Author Member

If there is a code that relied on this process (without explicitly calling StopAsync()), they should now explicitly call StopAsync().

@dahlia

dahlia approved these changes May 13, 2019

@longfin longfin merged commit dc9acff into planetarium:master May 13, 2019

5 checks passed

Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
codecov/patch 100% of diff hit (target 83.69%)
Details
codecov/project Absolute coverage decreased by -0.23% but relative coverage increased by +16.3% compared to 7410cbf
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.