Add fake clock for test code

[upgle]

Description

There is an issue that the test results are different depending on the performance of the build machine because the test code is written in a time-dependent. This PR improves memory queue test code that fails like heisenbug.

• Use fake clock to improve time-dependent test code execution time.
  • The memory queue test execution time was reduced from 7m3s -> 1m34s in downstream environment.
• Remove thread sleep and explicitly calls StateTimeout.

Related issue and scope

• I opened an issue to propose and discuss this change (#????)

My changes affect the following components

• Scheduler
• Tests

Types of changes

• Enhancement or new feature (adds new functionality).

Checklist:

• I signed an Apache CLA.
• I reviewed the style guides and followed the recommendations (Travis CI will check :).
• I added tests to cover my changes.
• My changes require further changes to the documentation.
• I updated the documentation where necessary.

[upgle]

-    -    Thread.sleep(5000)
+    +    clock.plusSeconds(5)

Change the thread sleep-based test to use fake clock.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Project coverage is 76.10%. Comparing base (1a0f1ce) to head (2890b1f).
Report is 99 commits behind head on master.

Files with missing lines	Patch %	Lines
...e/openwhisk/core/scheduler/queue/MemoryQueue.scala	90.47%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (1a0f1ce) and HEAD (2890b1f). Click for more details.

HEAD has 15 uploads less than BASE

Flag	BASE (1a0f1ce)	HEAD (2890b1f)
	20	5

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5304      +/-   ##
==========================================
- Coverage   81.24%   76.10%   -5.14%     
==========================================
  Files         238      239       +1     
  Lines       14178    14192      +14     
  Branches      579      580       +1     
==========================================
- Hits        11519    10801     -718     
- Misses       2659     3391     +732     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[style95]

How can this make sure the timeout happens after idleGrace and stopGrace respectively?

[upgle]

@style95

When idleGrace and stopGrace times out, a StateTimeout message is sent.
It explicitly sends a StateTimeout message, which is exactly the same as a message sent by timeout .

when(Running, stateTimeout = queueConfig.idleGrace) {
when(Idle, stateTimeout = queueConfig.stopGrace) {

[style95]

Yes but I feel the semantic is different.
For example, if anyone removes the stateTimeout configuration in each state in MemoryQueue by mistake, the previous code can guarantee that it should be added.

But the changed code cannot guarantee it.

[upgle]

@style95 I think what you said can be guaranteed with separate test code waiting for timeout.
For example, with a very short time timeout setting, the test code can wait as much as the actual timeout. I'll add some test code that covers what you said. What do you think?

[style95]

ok, that would be great.

[upgle]

@style95 Since I verified that the state timeout works normally in each state, I think that the test code in MemoryQueueFlowTests is already covered at the same level as before (StateTimeout is explicitly called). Is there anything I've missed?

[style95]

I meant what you added here: c1de659#diff-042a5fb7a94e1f4755cd38f9d3892ceed3db93e54cb7d34bd9bdf8289ef7c7fcR185
covers the timeout for the normal flow.
But there are many cases for other timeouts such as flushGrace and gracefulShutdownTimeout.

The MemoryQueueFlowTests guarantees the behavior of the memory queue in order, so I just want to make sure its coverage is not changed.

[upgle]

@style95
I've already covered idleGrace, stopGrace, gracefulShutdownTimeout you said. I'll add test for flushGrace as well. flushGrace seems to have been added recently.

• idleGrace
  • c1de659#diff-042a5fb7a94e1f4755cd38f9d3892ceed3db93e54cb7d34bd9bdf8289ef7c7fcR184
• stopGrace
  • c1de659#diff-042a5fb7a94e1f4755cd38f9d3892ceed3db93e54cb7d34bd9bdf8289ef7c7fcR188
• gracefulShutdownTimeout
  • c1de659#diff-042a5fb7a94e1f4755cd38f9d3892ceed3db93e54cb7d34bd9bdf8289ef7c7fcR192

According to the state timeout config for each FSM state, the actual time passes and the state is changed when timeout. I ensured that timeout works well in each state with the test code based on timer (thread sleep).

Because I tested sending FSM StateTimeout when real time passes, explicitly sending StateTimeout ensures the same level of test coverage.

[upgle]

@style95 A new test code is added for onTransition(Transition _ => Flushing). It checks if the timer StopQueue sending StateTimeout is active after the state transitions to Flushing.

https://github.com/apache/openwhisk/pull/5304/files#diff-042a5fb7a94e1f4755cd38f9d3892ceed3db93e54cb7d34bd9bdf8289ef7c7fcR255

    // Test case _ -> Flushing => startTimerWithFixedDelay("StopQueue", StateTimeout, queueConfig.flushGrace)
    // state Running -> Flushing
    expectMsg(Transition(fsm, Running, Flushing))
    fsm.isTimerActive("StopQueue") shouldBe true

And also, it checks whether StateTimeout msg is actually received by waiting for flush grace time.

[style95]

Great!

[jiangpengcheng]

LGTM