Cleanup #52
Merged
Cleanup #52
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…isync into Cleanup # Conflicts: # doc/dataStructures/CuckooFilters.md
…isync into Cleanup # Conflicts: # doc/dataStructures/CuckooFilters.md
…g (i.e. avoid strtok)
…isync into Cleanup
novakboskov
requested changes
May 28, 2020
| @@ -50,7 +50,7 @@ void CommSocketTest::SocketSendAndReceiveTest() { | |||
| //If socketSendReceiveTest() has finished successfully then kill child and break out of the loop | |||
| if(result == 0) { | |||
| CPPUNIT_FAIL("Sockets did not establish a connection in time"); | |||
| kill(timer_pid, 0); | |||
| // kill(timer_pid, 0); | |||
There was a problem hiding this comment.
We let the parent process to gracefully finish?
Can we delete this commented line?
| @@ -57,5 +57,8 @@ void CuckooSyncTest::setReconcileTest() { | |||
| // together because the execution trace picks up ZZ_p::init() call | |||
| // before it reaches syncTest helper function. | |||
| ZZ_p::init(randZZ()); | |||
| CPPUNIT_ASSERT(syncTest(client, server, false, false, false, false, false)); | |||
|
|
|||
There was a problem hiding this comment.
OK. I will look through it more thoroughly.
Just wondering, did this happen to you after I fixed Cuckoo::prng?
There was a problem hiding this comment.
don't know ... but I don't think so
|
On May 28, 2020, at 5:56 PM, 5/28/20, Novak Boškov ***@***.***> wrote:
@novakboskov requested changes on this pull request.
It looks good. Just a couple of questions.
In tests/unit/CommSocketTest.cpp <#52 (comment)>:
> @@ -50,7 +50,7 @@ void CommSocketTest::SocketSendAndReceiveTest() {
//If socketSendReceiveTest() has finished successfully then kill child and break out of the loop
if(result == 0) {
CPPUNIT_FAIL("Sockets did not establish a connection in time");
- kill(timer_pid, 0);
+ // kill(timer_pid, 0);
We let the parent process to gracefully finish?
Can we delete this commented line?
I think that it's dead code right now, so I don't see why not delete it.
In tests/unit/CuckooSyncTest.cpp <#52 (comment)>:
> @@ -57,5 +57,8 @@ void CuckooSyncTest::setReconcileTest() {
// together because the execution trace picks up ZZ_p::init() call
// before it reaches syncTest helper function.
ZZ_p::init(randZZ());
- CPPUNIT_ASSERT(syncTest(client, server, false, false, false, false, false));
+
OK. I will look through it more thoroughly.
Just wondering, did this happen to you after I fixed Cuckoo::prng?
I think it was independent ...
… —
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#52 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACM5LU5UZISSMV3AH7M75PLRT3M2FANCNFSM4NGK3MWQ>.
---
Ari Trachtenberg, Boston University
http://people.bu.edu/trachten mailto:trachten@bu.edu
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Random quote of the day:
"A pious Jew is not one who worries about his fellow man's soul and his own stomach, but one who worries about his own soul and his fellow man's stomach." - Rabbi Israel Salanter
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some cleanup of code style and efficiency ... more to come later.