test(recordings): Add recording options itests

[jan-law]

Related #474

Tests successful scenarios for the TargetRecordingOptionsGetHandler, TargetRecordingOptionsPatchHandler, and the TargetRecordingOptionsListGetHandler.

[andrewazores]

Tests look good. Do you still observe #263? Either way, I think it would be worth writing some tests to capture that behaviour.

[jan-law]

Do you still observe #263?

Yes, new recordings are created with toDisk=true.
Should recordings write to disk by default? If you'd like, I could change the default options to return toDisk=true.

[andrewazores]

The default should be whatever the target JVM says it is, which according to docs, seems it should be true. #263 seems to reflect a bug in JMC code, then, where the recording options map contains false when it would be expected to be true or perhaps null (unset, as the docs say).

[andrewazores]

Could you put a FIXME here with a link to #263? That's still something we should probably investigate further, although I am fairly convinced that it's just a JMC bug with how those OptionDescriptors are created or how the toDisk setting is read from the JVM.

[andrewazores]

Looks good otherwise.

[jan-law]

RecordingWorkflowIT failed at the part where it expects the in-memory jfr file to be larger than the saved jfr file. Why would it fail this time but pass most of the time? I'm re-running the tests to see what happens

Error:  Failures: 
Error:    RecordingWorkflowIT.testWorkflow:222 
Expected: a value greater than <9919>
but: <8305> was less than <9919>

[jan-law]

Hmm, now AutoRulesIT is failing because I forgot to reset the default recording options. Let me fix that

Error:  Failures: 
Error:    AutoRulesIT.testNewContainerHasRuleApplied:285 
Expected: <0>
     but: was <1000>

[andrewazores]

RecordingWorkflowIT failed at the part where it expects the in-memory jfr file to be larger than the saved jfr file. Why would it fail this time but pass most of the time? I'm re-running the tests to see what happens

Error:  Failures: 
Error:    RecordingWorkflowIT.testWorkflow:222 
Expected: a value greater than <9919>
but: <8305> was less than <9919>

Hmm. It sounds like it could simply be that the in-memory ring buffers filled up and some older event data was dropped in between the two recordings being retrieved, assuming that the recording isn't somehow configured with a maximum event age.

It's not a good fix, but changing the template in that test from ALL to just Continuous should cut down the number of events recorded and hopefully work around this edge case. Could you file a bug for this failure with a link to the CI log and whatever other findings you may have?

[andrewazores]

Hmm, now AutoRulesIT is failing because I forgot to reset the default recording options. Let me fix that

Error:  Failures: 
Error:    AutoRulesIT.testNewContainerHasRuleApplied:285 
Expected: <0>
     but: was <1000>

Aha - that would do it. maxSize=1000 in your 4th test case would be likely to cause the behaviour I described in the last comment, or at least cause it to occur much earlier/more frequently.