tests: add test for item trace #735
Conversation
Item Trace has some interesting behaviour: cancelling a previous trace doesn't take effect until the audience is garbage collected. I left this as there's no "unregisterListener" function already.
Codecov Report
@@ Coverage Diff @@
## main #735 +/- ##
============================================
+ Coverage 23.43% 23.47% +0.03%
- Complexity 10439 10458 +19
============================================
Files 1010 1010
Lines 158018 158037 +19
Branches 34938 34941 +3
============================================
+ Hits 37027 37092 +65
+ Misses 114284 114236 -48
- Partials 6707 6709 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I like this.
RequestLoggerOutput looks like a good simplification of the previous "custom stream" feature of RequestLogger, which I just discovered a few days ago.
And allowing Listeners to be unregistered is good new functionality. I looked at that when I wanted to created NamedListeners in my YouRobotManager test suite - for "(potions)" and "(avatar)" - and concluded I didn't need to clean them up since there was no harm if a freshly allocated Listener went out of scope and was cleaned up later - but I can easily envision a Cleanup that registers a NamedListener and unregisters it on close.
| var text = RequestLoggerOutput.stopStream(); | ||
| assertThat(text, containsString("ptrace: _VYKEACompanionType = couch")); | ||
| } | ||
| } |
There was a problem hiding this comment.
I notice that you are leaving the PreferenceListener active, rather than clearing it via execute("").
Intended?
There was a problem hiding this comment.
It was intended when I wrote it. The tests that clear it do so to test particular code paths.
But I think you're right, I should be clearing up the listeners. As it stands, whichever test runs last will leak its listeners into the remaining tests.
|
|
||
| String output = execute(""); | ||
| assertThat(output, containsString("Previously watched items have been cleared")); | ||
| } |
There was a problem hiding this comment.
This cleans up item listener at end of test.
There was a problem hiding this comment.
This is to cover the case where an item listener is unregistered without having been registered in the first place -- which happens if the item added has a negative id (i.e. is not a real item).
| var text = RequestLoggerOutput.stopStream(); | ||
| assertThat(text, containsString("itrace: hair spray = 1")); | ||
| } | ||
| } |
There was a problem hiding this comment.
This does not clean up item listener at end.
I changed it so you can trace any item, not just those currently in your inventory.
Item Trace had some interesting behaviour: cancelling a previous trace didn't take effect until garbage collection ran. I fixed this too.