search

jonboulle / clockwork

a fake clock for golang
Apache License 2.0
625 stars 57 forks source link

Fix tests after #44 is merged #48

Closed sagikazarmark closed 1 year ago

sagikazarmark commented 1 year ago

Fixes test failures introduced in #44

Signed-off-by: Mark Sagi-Kazar mark.sagikazar@gmail.com

sagikazarmark commented 1 year ago

@gagern can you please take a look at the breaking tests? I have no idea why CI wasn't triggered for #44, but it introduced a number of breaks. I fixed the first one, but the second one is not that trivial (the test passes locally, fails on CI).

sagikazarmark commented 1 year ago

@gagern any chance you could look into this?

There is also #49 that was reported for master.

I really-really don't want to, but I'll be forced to revert that awesome change if we can't figure these issues out. :/

DPJacques commented 1 year ago

I will take a look at this over the rest of the holiday break and see if I can come up with something.

I need to familiarize myself with what #44 added. I know it included AfterFunc and uses callbacks rather than channels, but I am unfamiliar with the particulars, specifically the "Otherwise it addresses a number of bugs, mainly around out-of-order or duplicate delivery of events, as exposed by the various tests added by these changes."

DPJacques commented 1 year ago

Figured it out. Fixes incoming. Pardon me as I am still figuring out Github.

sagikazarmark commented 1 year ago

Appreciate it!

DPJacques commented 1 year ago

Turns out this was mostly a timeout issue. I made #51 to address that and standardize a bit.

While I was going down the rabbit hole, I the control of faceClock.l challenging to follow. One can puzzle it out and eventually understand it, but I wanted it to be more evident to an uninformed reader. My goal was to make it easier to spot bugs and gotchas while hopefully making future contributions easier. Those changes are in #52 . Please let me know what you think.

51 and #52 are no-overlapping changes.

sagikazarmark commented 1 year ago

This is done