Add ARM specific coverage test

[andreeaflorescu]

Our CI is checking is the coverage was decreased by looking at the value in tests/coverage. Check test_coverage in test_coverage.py.

As coverage will be different on arm and x86 because of different ioctls and code paths that can be exercised on each platform, we should also have different coverage files and update the test to use different files for different platforms.

[andreeaflorescu]

I tried kcov on arm and it hangs. The cargo kcov command doesn't work at all.
Related issues:

• kcov issue: kcov hanging on arm64 SimonKagstrom/kcov#267
• cargo kcov issue: cargo kcov doesn't work on arm64 kennytm/cargo-kcov#40

[MrXinWang]

Hi @andreeaflorescu ! we are currently trying to contribute for Arm64 platform, and I think I will try to address this issue. Therefore I am wondering if I can try the integration test locally or must do it on buildkite?

Also I think the README in rust-vmm/rust-vmm-ci needs to be updated since the folder tests is not used anymore (I will do that together with this issue).

The idea is: Referencing #8 , I think we have already had a PR#62 to solve the run_code example for Arm64 and we also have unit tests for Arm64 now, so I think this is the only issue left for arm64 support?

[andreeaflorescu]

Hi @andreeaflorescu ! we are currently trying to contribute for Arm64 platform, and I think I will try to address this issue. Therefore I am wondering if I can try the integration test locally or must do it on buildkite?

You can try the test locally. You can run the command specified in Adaptive Coverage. For that to work you need to update the path of the coverage test. The new path, relative to the kvm-ioctls directory is rust-vmm-ci/integration_tests/test_coverage.py.

Also I think the README in rust-vmm/rust-vmm-ci needs to be updated since the folder tests is not used anymore (I will do that together with this issue).

Yap, that's correct.

The idea is: Referencing #8 , I think we have already had a PR#62 to solve the run_code example for Arm64 and we also have unit tests for Arm64 now, so I think this is the only issue left for arm64 support?

Once the arm code example is merged, that should be the case. I think we also need to add some more tests to arm though. If we can get the coverage, we can see what paths are not covered.

Did you manage to run kcov on arm? Did they recently added support for this? I am asking because when I tried it, kcov would exit with an error.

[MrXinWang]

The new path, relative to the kvm-ioctls directory is rust-vmm-ci/integration_tests/test_coverage.py.

absolutely, my bad, tried this on x86_64 instead, it works perfectly.

Did you manage to run kcov on arm? Did they recently added support for this? I am asking because when I tried it, kcov would exit with an error.

Thanks so much for your patience and fast reply! I think that kcov problem explains the reason why I can only run the integration test on my x86 machine (the test hangs on arm64)......I will try to further investigate this problem then. Thanks

[MrXinWang]

Also I remember in my last PR, problem with kcov was addressed by using python3 instead of python. I will try that too.

[MrXinWang]

Hi @andreeaflorescu !

Me and my colleague @michael2012z found a method to fix the kcov hang issue on arm64, and I think we still need some time before contributing our patch to the kcov master branch due to some legal limitations.

However I do tested the code coverage of the /kvm-ioctls repo, here are the results:

rust-vmm-ci/integration_tests/test_coverage.py F                                                                                                   [100%]

======================================================================== FAILURES ========================================================================
_____________________________________________________________________ test_coverage ______________________________________________________________________

profile = 'devel'

    def test_coverage(profile):
        coverage_config = _read_test_config()
        current_coverage = _get_current_coverage(coverage_config)
        previous_coverage = coverage_config["coverage_score"]
        if previous_coverage < current_coverage:
            if profile == pytest.profile_ci:
                # In the CI Profile we expect the coverage to be manually updated.
                assert False, "Coverage is increased from {} to {}. " \
                              "Please update the coverage in " \
                              "tests/coverage.".format(
                    previous_coverage,
                    current_coverage
                )
            elif profile == pytest.profile_devel:
                coverage_config["coverage_score"] = current_coverage
                _write_coverage_config(coverage_config)
            else:
                # This should never happen because pytest should only accept
                # the valid test profiles specified with `choices` in
                # `pytest_addoption`.
                assert False, "Invalid test profile."
        elif previous_coverage > current_coverage:
            diff = float(previous_coverage - current_coverage)
>           assert False, "Coverage drops by {:.2f}%. Please add unit tests for" \
                          "the uncovered lines.".format(diff)
E           AssertionError: Coverage drops by 18.20%. Please add unit tests forthe uncovered lines.
E           assert False

rust-vmm-ci/integration_tests/test_coverage.py:118: AssertionError

We will keep working on this.

[andreeaflorescu]

@MrXinWang wow! That's super nice!

If we manage to run kcov on arm as well, we probably need to think about a way to have different coverage files for arm and x86_64. This means we need to update the rust-vmm-ci.

Btw, can you share the coverage report? I am curious why does the coverage drop by 18.2%.

[MrXinWang]

Hi @andreeaflorescu !

If we manage to run kcov on arm as well, we probably need to think about a way to have different coverage files for arm and x86_64. This means we need to update the rust-vmm-ci.

Yes of course and I have prepared a patch to update the test_coverage.py and new coverage_config.json for arm64. However I am wondering if I need to do this for all repos under rust-vmm, and I think I do?

Btw, can you share the coverage report? I am curious why does the coverage drop by 18.2%.

Of course but how? You ok with the screenshots (probably too long)?

[andreeaflorescu]

Hi @andreeaflorescu !

If we manage to run kcov on arm as well, we probably need to think about a way to have different coverage files for arm and x86_64. This means we need to update the rust-vmm-ci.

Yes of course and I have prepared a patch to update the test_coverage.py and new coverage_config.json for arm64. However I am wondering if I need to do this for all repos under rust-vmm, and I think I do?

Well, I don't think we need to do that for all repositories. We can be inventive here and let coverage.json point to the x86_64 coverage and just add an optional coverage_aarch64.json file. We can discuss about this at length in rust-vmm-ci. Do you want to create an issue there or should I? We can discuss about how to move forward on that issue.

Btw, can you share the coverage report? I am curious why does the coverage drop by 18.2%.

Of course but how? You ok with the screenshots (probably too long)?

Can you maybe upload the cov directory somewhere and paste a link here? You might be able to attach it to this issue if you create an archive out of that coverage directory.

[MrXinWang]

@andreeaflorescu please refer to rust-vmm-ci/issue#10

[michael2012z]

Kcov hanging problem on aarch64 was fixed by SimonKagstrom/kcov#316 just now.
ARM coverage test should soon be unblocked.
@andreeaflorescu , @MrXinWang

[andreeaflorescu]

@MrXinWang would you like to update the rust-vmm-ci and add the coverage test for arm? I have a PR that updates the container to the previous version and fixes some tiny problems generated by switching to Rust 1.39. After that one is merged, we can add another update for the arm test. I can also update it directly in the existing PR. Let me know :)

[MrXinWang]

@andreeaflorescu Yes of course

[MrXinWang]

@andreeaflorescu I will try to solve the --no-clean-up thing for devel profile in that PR then. Would this OK to you?

[andreeaflorescu]

Sure. Sounds good. So should I update the rust-vmm-ci in kvm-ioctls to support arm coverage test, or would you want to do that?

[MrXinWang]

Sure. Sounds good. So should I update the rust-vmm-ci in kvm-ioctls to support arm coverage test, or would you want to do that?

Sorry I didn't notice this is in kvm-ioctl...I will do that then.

[andreeaflorescu]

The coverage test is available for aarch64, but at the moment it is ignored because it was flaky. This issue is tracked in: rust-vmm/rust-vmm-ci#18

Once we find a fix for this, the test will automatically be picked up by kvm-ioctls as well, so there is no need to track the problem in both places. Closing this.