Skip to content
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

Code coverage for monorepo #6478

Closed
mlewand opened this issue Mar 23, 2020 · 6 comments · Fixed by #6605
Closed

Code coverage for monorepo #6478

mlewand opened this issue Mar 23, 2020 · 6 comments · Fixed by #6605
Assignees
Labels
type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@mlewand
Copy link
Contributor

mlewand commented Mar 23, 2020

Provide a description of the task

A tricky case might be to do per-package code coverage after going for monorepo.

📃 Other details

This is a subtask of #6466.

@mlewand mlewand added the type:task This issue reports a chore (non-production change) and other types of "todos". label Mar 23, 2020
@mlewand mlewand self-assigned this Mar 23, 2020
@Reinmar Reinmar mentioned this issue Mar 24, 2020
7 tasks
@mlewand mlewand added this to the iteration 31 milestone Mar 25, 2020
@mlewand
Copy link
Contributor Author

mlewand commented Mar 27, 2020

Tentatively estimating it at 4 points, but it's a blind shot as first we need to clarify the requirements.

@mlewand
Copy link
Contributor Author

mlewand commented Apr 1, 2020

The most challenging fact about this task is to allow for per-package code coverage while moving to monorepo structure.

I'm getting promising results using codecov. While working on ckeditor5 repo sandbox copy I made a branch with just a few packages modified and the testing script correctly picked up which packages should be tested 👍 codecov command was run separately for each package (or flag using codecov nomenclature).

Now, there are still couple of things to do:

  • ensure that codecov provides some sort of message in pull requests, ensuring that the code coverage remains as high as it should
  • determine a proper reference branch - currently it always uses master branch as a reference to determine which packages should be changed. But sometimes it will be a stable branch for documentation changes.
  • adjust the scripts/codecov-run-tests.sh script so that it can be run for the master branch only (currently it was intended only to be run for branched builds)

@mlewand
Copy link
Contributor Author

mlewand commented Apr 1, 2020

I noticed a problem on master where sometimes not all tests are executed, thus there's no full code coverage.

Restarting travis build would eventually lead to full test run, and full code coverage. So clearly there's some integration problem.

The symptoms were that only part of tests were executed:

Executed 6652 of 13583 (skipped 15) SUCCESS (1 min 15.828 secs / 29.534 secs)

And there were several logs:

Detected `console.warn():', 'Selection change observer detected an infinite rendering loop

And I think this one bit is a key to the problem.

@mlewand
Copy link
Contributor Author

mlewand commented Apr 9, 2020

Today we decided to change a little our goals for CI.

The main goal remains to be able to tell the CC per package, but it's no longer a must-have for us to keep these results separate on codecov (a short backstory is that we wanted to pass per-package coverage using flags mechanism in codecov, but once there are 50+ flags it didn't reliably put nice bot messages to GH PRs).

Instead the per-package-coverage will be performed on Travis alone. And if any of the packages shows a lower code coverage then it should place a fail status in related PR.

Travis will execute test for each package separately (so a dedicated yarn run test -f <package> -c). After that code coverage data from each run should be combined into one big coverage data file, which will be sent to codecov for a nice GUI.

I already checked that combining json output works fine so we should be good here.

@mlewand
Copy link
Contributor Author

mlewand commented Apr 15, 2020

Ok, I created a script that does per-package testing and combines the results once all are done and uploads combined results to codecov.

The solution is ready for review in #6605

Failing tests

Each CKE5 package is tested separately.

Failing package suite doesn't stop others from running. It was important goal for me to make sure that all package suites are run - so that you get a full picture of what's wrong.

You can see an example build where some autoformat test cases are failing (also some packages are missing coverage).

For readabilty passing suites do not print the output.

Incomplete package coverage

Here's an example test run where table and cloud-services-core packages doesn't meet 100% CC.

It concludes it with a message "Following packages did not provide required code coverage: table cloud-services-core" at the end and marks the CI build as failed.

Note that the combined result uploaded to codecov show (almost because master branch that we used had incomplete tests for table) full coverage - because results uploaded to codecov are combined - missing parts were tested by other packages.

Timing

It takes ~13 minutes to finish the testing.

Codecov flags

As of today it doesn't make sense to use codecov's flags - it doesn't bring any value.

Even the docummentation says:

Flags feature on the Codecov Compare page

As of June 2019, Codecov has suspended flags for Codecov.io users on the Codecov Compare page.

@mlewand mlewand modified the milestones: iteration 31, iteration 32 Apr 22, 2020
@Reinmar Reinmar modified the milestones: iteration 32, iteration 31 May 1, 2020
@Reinmar
Copy link
Member

Reinmar commented May 1, 2020

I moved this back to it31 as a great majority of the work was done already. If you want to have a followup in it32, please report a new ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants