spock icon indicating copy to clipboard operation
spock copied to clipboard

Changed "eventually" usage example to reflect asynchronous logic

Open dlehammer opened this issue 6 years ago • 6 comments

I've attempted to make the usage example more transparent, as I've lost track on how many times I've been asked why the "machine" variable isn't (magically) updated by Spock. And I attempt to explain that the original Machine example seems to be thread-based.

Hopefully this change aids those who use PollingConditions to test asynchronous logic.


This change is Reviewable

dlehammer avatar Apr 12 '19 15:04 dlehammer

What makes the new example more clear in your opinion?

leonard84 avatar Jan 21 '20 19:01 leonard84

I was trying to make the current example more accessible, while trying to be concise. The idea was to intuitively illustrate that eventually doesn't (implicitly/magically) update the state that's attempted verified for a time period.

Instead the programmer must either

  • Explicitly get a new engine reading to evaluate via a getEngineReading() method every time the eventually-block is executed. (new example)
  • Implicitly await some logic outside the eventually-block updates the machine state, ex. thread-based asynchronous logic, so evaluating the state a later point in time might yield a different result. (current example)

Coming back to this PR I can see that perhaps it's not transparent that I'm assuming the reader will understand that there's a getEngineReading() method, hiding behind bean-notation, that provides the latest state.

Any suggestions?

dlehammer avatar Jan 22 '20 21:01 dlehammer

Codecov Report

Merging #990 into master will decrease coverage by 1.97%. The diff coverage is 84.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #990      +/-   ##
============================================
- Coverage     75.99%   74.01%   -1.98%     
+ Complexity     3544     3432     -112     
============================================
  Files           377      382       +5     
  Lines         10788    10612     -176     
  Branches       1374     1295      -79     
============================================
- Hits           8198     7855     -343     
- Misses         2109     2296     +187     
+ Partials        481      461      -20
Impacted Files Coverage Δ Complexity Δ
...g/spockframework/junit4/AbstractRuleExtension.java 86.66% <ø> (ø) 10 <0> (?)
.../java/org/spockframework/runtime/SpockRuntime.java 71.11% <ø> (+2.03%) 20 <0> (-1) :arrow_down:
...n/java/spock/util/concurrent/BlockingVariable.java 93.75% <ø> (ø) 6 <0> (ø) :arrow_down:
...kframework/spring/mock/SpockMockPostprocessor.java 85.03% <ø> (+1.03%) 41 <0> (-1) :arrow_down:
...in/java/spock/util/concurrent/AsyncConditions.java 93.54% <ø> (ø) 9 <0> (ø) :arrow_down:
...pock-core/src/main/java/spock/mock/MockingApi.java 2.94% <ø> (ø) 2 <0> (ø) :arrow_down:
spock-core/src/main/java/spock/lang/Retry.java 100% <ø> (ø) 0 <0> (ø) :arrow_down:
.../spockframework/report/log/ReportLogExtension.java 100% <ø> (+28.57%) 1 <0> (-7) :arrow_down:
...spockframework/runtime/SpockComparisonFailure.java 83.33% <ø> (ø) 3 <0> (ø) :arrow_down:
...spockframework/junit4/AbstractRuleInterceptor.java 85.71% <ø> (ø) 4 <0> (?)
... and 121 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 15e3169...3cfc27b. Read the comment docs.

codecov[bot] avatar Jan 22 '20 21:01 codecov[bot]

I also don't understand how the new example is any better than the old example. What does why the "machine" variable isn't (magically) updated mean? And how would the new example be clearer, it still uses the machine variable, so wouldn't the same question come up?

Vampire avatar Jan 23 '20 09:01 Vampire

Hi @Vampire,

What does why the "machine" variable isn't (magically) updated mean?

That's the question I so far always have to explain to my fellow developers, that's stumped by the current machine example vs. the actual behavior when eventually is introduced in a Spock specification.

I've seen a lot of code like the example below from new developers introduced to eventually via the documentation.

...
when:
	def result = myService.getAsyncResult()
then:
	pollingConditions.eventually {
		result.value == expected
	}
...

The above example ends up looping over the state from the initial invocation, and hence never succeeds.

In-order to get the desired behavior instead, I've approached it as follows.

...
expect:
	pollingConditions.eventually {
		def result = myService.getAsyncResult()
		result.value == expected
	}
...

I also don't understand how the new example is any better than the old example. .. And how would the new example be clearer, it still uses the machine variable, so wouldn't the same question come up?

Well, I was hoping it was possible to collaborate with the maintainers/community in-order to distill an example that doesn't need further explanation to an inexperienced Spock developer. Ie. how do we convey the power of eventually while simultaneously communicating what's expected by a new developer attempting to utilize eventually.

The current commit is my initial attempt from last year, I'm happy to update it based on suggestions.

dlehammer avatar Jan 23 '20 20:01 dlehammer

Well, this depends heavily on how the domain objects work. If Machine#getValue() changes what it returns once the asyn operation finished, the first example is perfectly fine.

If this is not the case and the first example is used, it imho only shows that the respective developer did not understand how the asynchronous operation works actually.

I don't think that the second example is any clearer or more correct, especially without information on how the domain object is actually working. So I'm with Leonard, that the new example is in no way any more helpful and I don't think that a simple example change can introduce the clarity you are after without explicitly describing how the domain objects behave.

Vampire avatar Jan 24 '20 09:01 Vampire