Skip to content

Enable scoverage on the test suite #25009

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.

Sign up for GitHub

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

Jump to bottom
Open
anatoliykmetyuk wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Enable scoverage on the test suite #25009

anatoliykmetyuk wants to merge 21 commits into from
+314 −17

Conversation

@anatoliykmetyuk
Copy link
Contributor

@anatoliykmetyuk anatoliykmetyuk commented Jan 19, 2026

This PR enables scoverage phase on the compiler tests. The purpose is to check if the compiler crashes with this flag enabled on some of the tests.

The implementation proposed in this PR runs pos tests with the scoverage flag in a separate CI job. Currently, with this flag enabled, 26 tests fail. The exact checks executed by the test:

  • Can the compiler compile the tests without crashing?
  • Is the scoverage output file produced as a result of the compilation?
  • Is the scoverage output file correctly formatted, i.e. can it be deserialized?

Further work:

  1. Increase testing surface: enable scoverage on more tests - e.g. neg, run, standard library.
  2. Implement a mechanism to filter out failing tests. This would allow to merge the testing harness and preserve the coverage of the currently passing tests.
  3. Fix the discovered failing tests in the compiler, removing them from the filtering mechanism (2).

cc @smarter

@anatoliykmetyuk anatoliykmetyuk marked this pull request as ready for review January 28, 2026 01:28
@anatoliykmetyuk anatoliykmetyuk force-pushed the enable-scoverage branch 2 times, most recently from 6577a5c to 1b9f906 Compare January 29, 2026 23:03
@anatoliykmetyuk
Copy link
Contributor Author

anatoliykmetyuk commented Jan 30, 2026

@smarter this PR is mature enough so could you please review it? It would be good to focus on merging this PR with the support of scoverage on a basic test surface (pos tests, run tests and a few others) before expanding the test surface or addressing the failing issues.

Here's a brief description of changes:

@Gedochao Gedochao requested review from smarter and tgodzik February 12, 2026 16:19
@Gedochao
Copy link
Contributor

Gedochao commented Feb 12, 2026

@anatoliykmetyuk can you rebase? it seems there are conflicts.
Not sure if @smarter is available, so I asked @tgodzik to take a look.

tgodzik
tgodzik requested changes Feb 12, 2026
View reviewed changes
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions and minor comments from me. We also need to rebase

compiler/test/dotty/tools/dotc/CoverageSupport.scala
val fileMatches = sourceFiles.exists(matchesIgnoreList)

// Also check the basename of the test title (covers SeparateCompilationSource directory targets)
val titleMatches = ignoreList.contains(new java.io.File(target.title).getName)
Copy link
Contributor

@tgodzik tgodzik Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we beed this? Which files in the exclude list are handled by this?

Copy link
Contributor Author

@anatoliykmetyuk anatoliykmetyuk Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The titleMatches check is needed for directory tests where source files are in nested subdirectories. For example, i15845 contains only a config/ subdirectory (no direct .scala files), so SeparateCompilationSource.sourceFiles returns an empty array. Since there are no files to iterate, fileMatches (including parentMatch) is always false. Only titleMatches catches these by extracting the directory name from the test title.

Concrete examples from the excludelist that rely on titleMatches:

  • i15845 — contains only config/ subdirectory
  • i15608 — contains only bar/, foo/ subdirectories
  • i15980 — contains only scalaql/ subdirectory

anatoliykmetyuk and others added 18 commits February 13, 2026 08:40
@anatoliykmetyuk
Enable scoverage on positive compilation tests
f92e593
@anatoliykmetyuk
Add coverage test to CI
1ae78aa
@anatoliykmetyuk
Remove dead code
2598fbd
@anatoliykmetyuk
Add excludelist for scoverage tests
2395a7a
@anatoliykmetyuk
Add failing pos tests to excludelist
14ec65a
@anatoliykmetyuk
Stricting the scoverage CI pos tests filter
05fa266
@anatoliykmetyuk
Proper directory exclusion logic
50e8885
@anatoliykmetyuk
Add runTests to scoverage testing surface
8ce477c
@anatoliykmetyuk
Enhance explicit nulls test to support coverage instrumentation
4422d77
@anatoliykmetyuk
Add coverage support to compilation tests and refactor test execution…
a899e9b 
… logic

- Introduced `CoverageSupport` trait to handle coverage file verification and instrumentation.
- Updated `BootstrappedOnlyCompilationTests` and `CompilationTests` to utilize coverage instrumentation when running tests.
- Refactored test execution logic to conditionally check for coverage based on the `Properties.testsInstrumentCoverage` flag.
- runMacros, runWithCompiler, runBootstrappedOnly, genericJavaSignatures support coverage testing
@anatoliykmetyuk
Exclude more tests failing scoverage
1e0fa0c
@anatoliykmetyuk
Enable coverage tests on all tests that support that flag
7f5984f
@anatoliykmetyuk
Add more failng tests to exclusion list
dbcb602
@anatoliykmetyuk
Fix exclusion logic to handle all corner cases
acb3517
@anatoliykmetyuk @tgodzik
Update compiler/test/dotty/tools/dotc/CoverageSupport.scala
020d3a6 
Co-authored-by: Tomasz Godzik <tgodzik@users.noreply.github.com>
@anatoliykmetyuk @tgodzik
Update compiler/test/dotty/tools/dotc/CoverageSupport.scala
83d2793 
Co-authored-by: Tomasz Godzik <tgodzik@users.noreply.github.com>
@anatoliykmetyuk @tgodzik
Update compiler/test/dotty/tools/dotc/CoverageSupport.scala
ee91c61 
Co-authored-by: Tomasz Godzik <tgodzik@users.noreply.github.com>
@anatoliykmetyuk @tgodzik
Update compiler/test/dotty/tools/dotc/CoverageSupport.scala
e515b75 
Co-authored-by: Tomasz Godzik <tgodzik@users.noreply.github.com>
@anatoliykmetyuk
Copy link
Contributor Author

anatoliykmetyuk commented Feb 13, 2026

@tgodzik thank you for the review! I've addressed the review and rebased to the latest main. The CI failures appear to be flaky tests not related to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgodzik tgodzik tgodzik requested changes

@smarter smarter Awaiting requested review from smarter

Requested changes must be addressed to merge this pull request.

Assignees

@smarter smarter

@tgodzik tgodzik

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@anatoliykmetyuk @Gedochao @tgodzik @smarter