You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 1518c0f
Browse filesBrowse the repository at this point in the historyBrowse files
fix(ci): stop swallowing PHPUnit failures in docker-test workflow
Remove `2>/dev/null` and `|| echo …` from the PHPUnit step in
.github/workflows/docker-test.yml so a non-zero exit code correctly
fails the CI run. Add regression tests for the two crash paths
exposed by strict_types:
- tests/unit/CommonBackendLibraryTest.php (13 tests): buildSeoData()
with object keywords, invalid JSON, invalid UTF-8, empty fields,
XSS stripping, and getDatatablesPagination() edge cases.
- tests/unit/CommonTagsLibraryTest.php (10 tests): checkTags() with
invalid JSON (TypeError on foreach(null)), the data-loss scenario
where isUpdate=true deletes pivots before crashing, empty values,
XSS stripping, and normal CRUD flows via CommonModel mock.
Copy file name to clipboardExpand all lines: CHANGELOG.md
+4Lines changed: 4 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -20,6 +20,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
20
20
21
21
-**Sitemap Joined Language Rows on the Wrong Column, Dropping and Mis-Attributing URLs:**`App\Models\PagesModel::sitemapItems()` and `App\Models\BlogModel::sitemapItems()` joined `pages_langs` / `blog_langs` with `ON pages_langs.id = pages.id` — matching the *language row's own primary key* against the content id instead of using the `pages_id` / `blog_id` foreign key. The effect on live data was that the sitemap silently published a subset of URLs and attached some of them to the wrong record: for pages, 2 of 4 localized URLs never appeared at all and `pages.id=2` was advertised under page 1's slug; for blog, 3 of 6 appeared and 2 of those 3 pointed at the wrong post. Both joins now use the real foreign key, and rows whose `seflink` is empty or NULL are skipped rather than emitting a bare `/` or `/blog/` entry. Verified by running both the old and the new join against the live database and comparing the emitted URL sets.
22
22
23
+
-**CI PHPUnit Step Silently Swallowed Test Failures:**`.github/workflows/docker-test.yml` ran PHPUnit with `2>/dev/null || echo "⚠️ No tests defined or tests skipped."`, which suppressed stderr and masked any non-zero exit code — a failing test suite still produced a green CI run. Removed both the `2>/dev/null` redirect and the `|| echo …` fallback so a PHPUnit failure now correctly fails the workflow step. The original intent was to tolerate an empty test suite; with real tests in place, the escape hatch's purpose is served and its risk (silent regressions reaching `master`) outweighs its convenience.
24
+
25
+
- **Regression Tests for `CommonBackendLibrary::buildSeoData()` and `CommonTagsLibrary::checkTags()` — the Two Crash Paths Exposed by `strict_types`:** Added `tests/unit/CommonBackendLibraryTest.php` (13 tests) and `tests/unit/CommonTagsLibraryTest.php` (10 tests) targeting the exact code paths that previously crashed. `buildSeoData()` tests cover: a JSON-object keywords field (not an array) that bypassed the `is_array()` guard; invalid JSON keywords; invalid UTF-8 in the description field that caused `json_encode()` to return `false`; empty/whitespace-only fields; XSS tag stripping; and the `getDatatablesPagination()` helper. `checkTags()` tests use a `CommonModel` mock (injected via reflection) to verify: invalid JSON causing a `TypeError` on `foreach(null)` — specifically the scenario where `isUpdate=true` deletes pivot rows *before* the crash, losing tag relations; empty-string input; JSON object instead of array; empty-value tag skipping; XSS stripping; and normal create/update flows. The crash-path tests document **current broken behaviour** (`expectException(TypeError)`) and will need their expectations updated when the underlying methods are hardened.
26
+
23
27
### Changed
24
28
25
29
- **`declare(strict_types=1)` Extended to 48 Logic Files:** Coverage went from 28 to 76 first-party files. The declaration was added only to `Libraries/`, `Models/`, `Commands/` and `Filters/` — the layers that hold logic rather than presentation — and deliberately **not** to `Views/`, `Language/` or `app/Config/`. The exclusion is not stylistic: `strict_types` governs every function call *made from* the declaring file, and views are precisely where MySQL/MariaDB's all-columns-are-strings behaviour meets typed builtins, so a blanket rollout would convert working pages into `TypeError`s. Language files (`return [...]`) and config property bags gain nothing. Each batch was applied and verified against the test suite separately, and every target's call sites were checked for coercion hazards first — the `$builder->limit()` calls in `Ci4ms`, `AjaxModel` and `UserscrudModel` were confirmed safe because those methods already declare `int $limit` / `int $skip`. The two code-generator templates under `modules/Backend/Commands/Views/` were left untouched so the declaration does not leak into generated modules.
0 commit comments