kit icon indicating copy to clipboard operation
kit copied to clipboard

/events: Move `WithClock` to `unit` build tagged file

Open JoshVanL opened this issue 2 years ago • 5 comments

Reverts https://github.com/dapr/kit/pull/62

WithClock should be moved to unit build tagged files to not pollute implementation non-test builds. Developers wanting to override the clock in testing can do so by go testing with the unit build tag.

JoshVanL avatar Oct 12 '23 09:10 JoshVanL

Codecov Report

All modified lines are covered by tests :white_check_mark:

:exclamation: No coverage uploaded for pull request base (main@a043330). Click here to learn what that means.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #63   +/-   ##
=======================================
  Coverage        ?   81.05%           
=======================================
  Files           ?       40           
  Lines           ?     2328           
  Branches        ?        0           
=======================================
  Hits            ?     1887           
  Misses          ?      317           
  Partials        ?      124           
Files Coverage Δ
events/batcher/batcher.go 87.50% <ø> (ø)
events/batcher/unit.go 100.00% <100.00%> (ø)
events/queue/processor.go 90.21% <100.00%> (ø)
events/queue/processor_unit.go 100.00% <100.00%> (ø)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Oct 12 '23 09:10 codecov-commenter

#62 was made on purpose. Please don’t reverse this.

I don't see the technical reason for needing WithClock in the implementation code. The mock code is still available in tests with the unit build tag.

JoshVanL avatar Oct 12 '23 16:10 JoshVanL

@JoshVanL please resolve the conflict

yaron2 avatar Nov 15 '23 18:11 yaron2

@yaron2 Done

JoshVanL avatar Nov 15 '23 18:11 JoshVanL

#62 was made on purpose. Please don’t reverse this.

I don't see the technical reason for needing WithClock in the implementation code. The mock code is still available in tests with the unit build tag.

@ItalyPaleAle please review this comment so we can decide how to move forward with this PR

yaron2 avatar Nov 16 '23 17:11 yaron2