Add unit tests for main.go and handle APM server edge behaviors

[jlvoiseux]

Motivation

The goal of these tests is to ease the refactor of the extension following the recent code reviews. They can also be used to solve some of the current issues. For example :

• TestFlush reproduces "panic: send on closed channel" from the extension #133 (a RuntimeDone signal is sent to the extension after the closure of the channel)
• TestSeveralTransactions reproduces panic: sync: WaitGroup is reused before previous Wait has returned #128 (race condition causing a WaitGroup to be modified while it hangs)

The core idea is to combine a mock Lambda API backend with a mock APM server to enable the recreation of several scenarios.

Limitations

• Since Address partial list of code review comments #132 is not merged yet, TestFlush panics. Same for TestSeveralTransactions, as the related issue is not yet solved.
• Another limitation is the fact that due to the concurrent nature of the processes tested, the aforementioned failing tests might sometimes pass.

[ghost]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-03-15T15:31:14.363+0000

• Duration: 7 min 46 sec

Test stats 🧪

Test	Results
Failed	0
Passed	168
Skipped	6
Total	174

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[estolfo]

This is really great, thanks for building this test framework! I'm just wondering if there's some way we can add an assertion to the tests or if it makes sense to have the absence of a panic indicate that the test passes.

[estolfo]

Why did you move this block to after the Shutdown event check?

[jlvoiseux]

When writing my tests, I found myself in the following situation :

Test setup :

• APM Server configured to be very slow (approx 3 seconds to respond).
• APM Agent Data channel filled with data
• SendStrategy configured to background

Behavior :
The extension starts sending data from the loaded channel to the slow APM server. It is then interrupted due to the current event being finished. A shutdown event is then received, and we start flushing APM data. The issue is that this takes a tremendous time if the APM Agent data channel is full and if the response time of the Server is large, even if the SendStrategy is set to background.

An alternative solution would be to put the flush statement above the Shutdown, but to only run FlushAPMData if the SendStrategy is SyncFlush

[estolfo]

Thanks for the explanation. I think we might want to remove this flush anyway: with syncFlush, we'll flush at the end of an invocation so there won't be anything to flush upon a new function invocation. With background, we'll just continue receiving from the channel in the go routine when the extension starts its work for a new function invocation.

[jlvoiseux]

Regarding success/failure assertions, it really depends on the specific test. Explicitly stating an assertion for each test will increase clarity, even if it is just the absence of panic. I will make a proposal for each of them.

[jlvoiseux]

I added success/failure assertions in 41a8039. For most tests, I check if the extension does not panic and if APM data is correctly sent to the APM server.

[estolfo]

👍🏼

[estolfo]

Just two minor comments then I think we can merge this. Thanks for your work on this!

[jlvoiseux]

@estolfo Thank you for the review ! Your two suggestions have been addressed in 0f58cb5