Skip to content

Include controllers/apps in unit test coverage#1156

Open
urbanikb wants to merge 1 commit into3scale:masterfrom
urbanikb:add-e2e-build-tag-to-suite
Open

Include controllers/apps in unit test coverage#1156
urbanikb wants to merge 1 commit into3scale:masterfrom
urbanikb:add-e2e-build-tag-to-suite

Conversation

@urbanikb
Copy link
Copy Markdown
Contributor

@urbanikb urbanikb commented Apr 7, 2026

What

Since the make test-unit target is the only target we currently scrape coverage from (due to e2e test being run by prow and not sending data to codecov), it would be good if the unit tests in controllers/app would also run there.

Currently there are no unit tests defined, but there might be in the future PR.

How

  • Move the test code that requires envtest out of directory structure for operator code
  • Rename e2e test to integration test and add Makefile target

Verification

Checked that makefile targets work as before.
Ran the test-integration test locally against the Fyre cluster, it ran as usual.

@urbanikb urbanikb marked this pull request as ready for review April 7, 2026 21:11
@urbanikb urbanikb requested a review from a team as a code owner April 7, 2026 21:11
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 7, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.84%. Comparing base (4e62f84) to head (ea2f05e).
⚠️ Report is 27 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1156      +/-   ##
==========================================
+ Coverage   39.34%   41.84%   +2.49%     
==========================================
  Files         205      203       -2     
  Lines       23363    20859    -2504     
==========================================
- Hits         9193     8729     -464     
+ Misses      13184    11350    -1834     
+ Partials      986      780     -206     
Flag Coverage Δ
unit 41.84% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
apis/apps/v1alpha1 (u) 58.55% <ø> (+29.45%) ⬆️
apis/capabilities/v1alpha1 (u) 3.50% <ø> (+1.99%) ⬆️
apis/capabilities/v1beta1 (u) 20.21% <ø> (-1.16%) ⬇️
controllers (i) 9.31% <0.00%> (-0.02%) ⬇️
pkg (u) 61.71% <81.13%> (+0.72%) ⬆️
see 8 files with indirect coverage changes
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tkan145
Copy link
Copy Markdown
Contributor

tkan145 commented Apr 9, 2026

As the test is not really e2e, can we have another folder called /test/integration instead? Then add a new entry inside Makefile to run the integration test

@urbanikb
Copy link
Copy Markdown
Contributor Author

urbanikb commented Apr 9, 2026

As the test is not really e2e, can we have another folder called /test/integration instead? Then add a new entry inside Makefile to run the integration test

Just to rephrase what you are asking is to rename e2e to integration, is this correct? There is no "other" e2e test that would be left.

@tkan145
Copy link
Copy Markdown
Contributor

tkan145 commented Apr 13, 2026

Yes correct, and we need a new entry in the Makefile for the integration tests, I would also like to keep make test-e2e but pointing to the e2e directory, which will be empty so the test will return early. Once mergerd, I will update prow to run both e2e (current) and integration.

…verage

- Move e2e test out of the package for ordinary code
- Rename e2e test to integration and add makefile target

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@urbanikb urbanikb force-pushed the add-e2e-build-tag-to-suite branch from b5ada1b to adac37a Compare April 13, 2026 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants