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

[tests] Force tick on FwdEventTicker #1975

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

halseth
Copy link
Contributor

@halseth halseth commented Sep 25, 2018

Ensures events are properly flushed on FwdEventTick. Should also help with the flaky coverage reports.

@halseth halseth force-pushed the fwdtrigger-force branch 2 times, most recently from dbf5d6c to e2527b9 Compare September 25, 2018 09:31
@halseth halseth changed the title [tests] Force tick on FwdEventTrigger [tests] Force tick on FwdEventTicker Sep 25, 2018
@halseth halseth added this to the 0.5.2 milestone Sep 25, 2018
@halseth halseth added testing Improvements/modifications to the test suite travis Modifications to the Travis CI system tests labels Nov 1, 2018
cfromknecht
cfromknecht previously approved these changes Dec 2, 2018
Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

htlcswitch/switch_test.go Outdated Show resolved Hide resolved
htlcswitch/switch_test.go Outdated Show resolved Hide resolved
bobLog.Lock()
t.Fatalf("expected 5 events in event log, instead "+
"found: %v", spew.Sdump(bobLog.events))
bobLog.Unlock()
Copy link
Contributor

@wpaulino wpaulino Dec 20, 2018

Choose a reason for hiding this comment

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

Perhaps this should be deferred? The t.Fatal call shouldn't allow it to get to this point.

EDIT: not that it matters much I guess, since the tests will halt anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, good catch :)

Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@cfromknecht cfromknecht merged commit 75ec66d into lightningnetwork:master Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Improvements/modifications to the test suite tests travis Modifications to the Travis CI system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants