Skip to content

Comments

11.0.5#476

Open
iobrado wants to merge 10 commits intomainfrom
feature/ci-fixes
Open

11.0.5#476
iobrado wants to merge 10 commits intomainfrom
feature/ci-fixes

Conversation

@iobrado
Copy link
Contributor

@iobrado iobrado commented Feb 11, 2026

Description

Fixed

  • PHPStan errors.
  • added correct I18n classes to PHPUnit exclusions.

Updated

  • increased code coverage.
  • CI now uses PHP 8.4

Note: I leveraged Claude to help me out with the tests. The errors are fixed and the coverage is increased, but if you notice any issues with the updates made to the tests, let me know.

Before

Classes:  8.86% (7/79)
Methods: 10.41% (51/490)
Lines:   11.02% (442/4012)

After

Classes: 12.66% (10/79)
Methods: 13.99% (68/486)
Lines:   18.19% (729/4008)

After after

Classes: 20.25% (16/79)
Methods: 25.70% (119/463)
Lines:   23.26% (922/3964)

Even more after

Classes: 24.05% (19/79)
Methods: 30.67% (142/463)
Lines:   36.45% (1445/3964)

Final after v1

Classes: 31.65% (25/79)
Methods: 38.88% (180/463)
Lines:   44.11% (1744/3954)

@iobrado iobrado requested a review from a team February 11, 2026 12:00
@iobrado iobrado self-assigned this Feb 11, 2026
@iobrado iobrado added improvement Small improvement fixes, either readability or performance improvements tests Issue regarding tests labels Feb 11, 2026
@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.18%. Comparing base (419e3a2) to head (aa68feb).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files
@@              Coverage Diff              @@
##               main     #476       +/-   ##
=============================================
+ Coverage     15.24%   45.18%   +29.93%     
+ Complexity     1201     1138       -63     
=============================================
  Files            82       79        -3     
  Lines          4519     4300      -219     
=============================================
+ Hits            689     1943     +1254     
+ Misses         3830     2357     -1473     
Flag Coverage Δ
unittests 45.18% <100.00%> (+29.93%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@piqusy piqusy left a comment

Choose a reason for hiding this comment

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

Nice work 👏🏼

Left a few suggestions
Also, can you write tests for t... oh wait 🤣

Comment on lines 558 to 559
$this->assertStringContainsString(' disabled', $result);
$this->assertStringContainsString(' readonly', $result);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the empty space intentional here e.g. ' disabled'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't seem like it. Removed the empty space and I'm not seeing any difference in coverage


// Note: Can't easily mock static class methods with Brain Monkey
// When cache file has invalid JSON, the code throws InvalidManifest exception
// TODO: Consider if this should be caught and fallback to rebuilding cache
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the TODO valid here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be that Claude made some reminders to itself 😂

@iobrado
Copy link
Contributor Author

iobrado commented Feb 11, 2026

@piqusy you reviewed this professionally. I will leave you a ⭐⭐⭐⭐⭐ rating.

I addressed the concerns and added some additional tests, further increasing the coverage.

@iobrado iobrado requested a review from piqusy February 11, 2026 14:35
dadadavorin
dadadavorin previously approved these changes Feb 11, 2026
piqusy
piqusy previously approved these changes Feb 11, 2026
@iobrado iobrado dismissed stale reviews from piqusy and dadadavorin via 717dd50 February 12, 2026 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Small improvement fixes, either readability or performance improvements tests Issue regarding tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants