Skip to content

Proposal to investigate: Increase test coverage using integration tests #1829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
cmaglie opened this issue Aug 9, 2022 · 1 comment · Fixed by #2103
Closed
3 tasks done

Proposal to investigate: Increase test coverage using integration tests #1829

cmaglie opened this issue Aug 9, 2022 · 1 comment · Fixed by #2103
Assignees
Labels
conclusion: resolved Issue was resolved topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement

Comments

@cmaglie
Copy link
Member

cmaglie commented Aug 9, 2022

Describe the request

Currently, the code coverage is calculated only on unit-test because go-lang offers this feature via go test command. For example, in our go:test Taskfile script we usa a command line flag that can be summarized as:

$ go test -coverpkg=./... -covermode=atomic -coverprofile=coverage_unit.txt ...

These flags produce a coverage_unit.txt file that is later sent to the codecov.

In the integration tests, instead, we run directly a build of the arduino-cli, using the executils library. in this case, just adding the -cover* flags in the go test commands actually would not add any coverage (except maybe for the executils library).

BTW a way to workaround this limitation exists, even if a bit convoluted, here's how:

  1. the go test command offers a -c flag that "compiles" the test instead of running it:
    -c
        Compile the test binary to pkg.test but do not run it
        (where pkg is the last element of the package's import path).
        The file name can be changed with the -o flag.
    
    This means that we can create and executable that when run performs the tests exactly as go test will do.
  2. We can make a special "unit-test" that just runs the CLI with the given arguments, something along the line of:
    func RunArduinoCLITest(t *testing.T) {
       // ...mangle os.Args to fix some paths or arguments...
       cli.Main(os.Args)
    }
  3. We can create with go test ./internal/cli-runner/ --run=RunArduinoCLITest -coverpkg=./... -covermode=atomic -coverprofile=coverage_unit.txt -c -o arduino-cli-withcoverage an executable called arduino-cli-withcoverage that will run the arduino-cli as it was in a test but with the parameters we pass to it.
  4. We can use this arduino-cli-withcoverage to run integration tests instead of arduino-cli and get the coverage from the integration tests!

Now this is very theoretical, I can already see some obstacles:

  1. We need to refactor the arduino-cli so it has a callable Main method (easy)
  2. We need to mangle os.Args in the RunArduinoCLITest to make it digestible from the arduino-cli (medium)
  3. The test coverage output is written if the process terminates "cleanly": this means that we should intercept panics (medium) or os.Exit calls (hard)
  4. From the integration-test we may need to kill the arduino-cli-withcoverage process, but killing it would not produce coverage test, so we probably need to set up a communication channel to exit the process cleanly (hard)
  5. Each run of arduino-cli-withcoverage will produce a different coverage.txt, all the coverage.txt should be collected and merged together (medium)
  6. Something else that I'm surely not considering here (hard)

Anyway, given the difficulties above, IMHO it's still something worth considering if possible.

Describe the current behavior

No test coverage is reported from integration-tests.

Arduino CLI version

N/A

Operating system

N/A

Operating system version

N/A

Additional context

No response

Issue checklist

  • I searched for previous requests in the issue tracker
  • I verified the feature was still missing when using the nightly build
  • My request contains all necessary details
@cmaglie cmaglie added the type: enhancement Proposed improvement label Aug 9, 2022
@cmaglie cmaglie changed the title proposal to ivestigate: Increase test coverage using integration tests Proposal to investigate: Increase test coverage using integration tests Aug 9, 2022
@cmaglie cmaglie self-assigned this Aug 9, 2022
@cmaglie cmaglie added the topic: CLI Related to the command line interface label Aug 9, 2022
@per1234 per1234 added topic: infrastructure Related to project infrastructure and removed topic: CLI Related to the command line interface labels Aug 9, 2022
@cmaglie
Copy link
Member Author

cmaglie commented Mar 9, 2023

The proposal above is obsoleted by the more simple go build -cover introduced in golang >=1.20:

https://go.dev/blog/integration-test-coverage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: resolved Issue was resolved topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants