Embed test files - Githubissues

samarth-aws commented 1 month ago

Description of changes: Tests for the different analyses perform I/O to read from files which can cause errors and prevent the tests from running in parallel.

This PR avoids performing I/O in tests by embedding the test files into the test binaries via the embed package.

Notable changes:

Move test files for each package to the package's own "testdata" directory, instead of everything residing in "./testdata/src/". This is necessary because of a limitation in the embed package.
The backtrace tests that require files from the taint analysis are now in the taint_test package
Refactor analysistest.LoadTest to use the fs.FS interface instead of performing file I/O directly
Avoiding file I/O in tests allows them to be run in parallel
Remove go.mod files from all the test directories because these files cannot be embedded via the embed package

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

victornicolet commented 1 month ago

I see that it moves all testdata files into their respective packages, but then backtrace_test now needs to be in the taint package. That doesn't seem to be optimal to me?

samarth-aws commented 1 month ago

I see that it moves all testdata files into their respective packages, but then backtrace_test now needs to be in the taint package. That doesn't seem to be optimal to me?

It's not ideal but the embed package only supports embedding files in the same directory which is why I couldn't keep the existing testdata/src/<package> style. We could maybe have makefile rules (e.g. make taint-test) to run the tests in the taint package excluding the backtrace tests?

An alternative is to copy over the taint/testdata directory to the backtrace directory, but that also seems sub-optimal to me.

victornicolet commented 1 month ago

@amzn-jasonrk do you have an opinion on this? I was hoping the change to embedding would not change the testing files structures that is clearly separate from the source code (and makes working with complex tests in an IDE easier).

amzn-jasonrk commented 1 month ago

Is this the only way to fix the flaky tests? It seems like there is something deeper that we should fix, especially if this is only happening on one machine. Have we confirmed the root cause analysis of the bug?

samarth-aws commented 1 month ago

Is this the only way to fix the flaky tests? It seems like there is something deeper that we should fix, especially if this is only happening on one machine. Have we confirmed the root cause analysis of the bug?

I wouldn't say the tests are "flaky" since they fail consistently in Terminal.app but pass consistently in the GoLand terminal. I found the root cause of the issue: https://github.com/awslabs/ar-go-tools/issues/67

I still would like to merge this PR since doing file I/O in tests isn't great in general.

awslabs / ar-go-tools

Embed test files #66