fix: repair failing CI tests and critical npm audit advisories#1433
Merged
Conversation
Allow ApplicationController::renderProjectForRequest to handle studies and datasets without a parent project instead of raising a TypeError. Treat empty-string text columns as missing in the molecule enrichment query since molecular_formula is NOT NULL in the schema. Disable the cookie consent middleware in tests because it rewrites response HTML and breaks Inertia assertions. Align the project role and workspace tests with owner-first role resolution, and fake nested Artisan calls with a proxy partial mock so the outer command still runs.
Override shell-quote to >=1.8.4 (GHSA-w7jw-789q-3m8p) and bump the instantsearch.js qs override to 6.15.2 (GHSA-q8mj-m7cp-5q26) so the npm audit high-severity gate passes again.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #1433 +/- ##
==================================================
+ Coverage 56.63% 75.78% +19.14%
- Complexity 2268 3446 +1178
==================================================
Files 207 240 +33
Lines 8733 12912 +4179
==================================================
+ Hits 4946 9785 +4839
+ Misses 3787 3127 -660
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CI on
developmenthas been red for a while: the Tests & Coverage (PHP 8.4) job fails with 3 errors + 8 failures, and the JS Lint & Security job started failing onnpm audit --audit-level=highonce new advisories were published. This PR fixes all of them. These same failures also bleed into every open PR's checks (e.g. #1425).Test fixes
ApplicationController::renderProjectForRequestnow accepts a nullable project. Studies and datasets published without a parent project previously hit aTypeError(null passed to aProjecttype hint); they now renderPublic/Sample/Show/Public/Sample/Datasetas the tests expect, with a 404 guard for project-level tabs without a project.MoleculeEnrichmentInspector::needingEnrichmentQuerytreats empty strings as missing for text columns.molecules.molecular_formulais NOT NULL in the schema (stored as''when unknown), so the oldwhereNullchecks could never match and the tests insertingnullviolated the constraint. Tests updated to use''.COOKIE_CONSENT_ENABLED=falseinphpunit.xml). Spatie's middleware rewrites the response HTML viasetContent(), which replaces the originalViewobject with a string and makes Inertia'sassertInertia()always fail with "Not a valid Inertia response". This unblocksTextSearchPageTestand the compounds-scope test (and likely explains why this repo grew the manualassertInertiaPageComponenthelper).RepairMissingCompoundInfoCommandTestfakes the nestedArtisan::callwith a proxy partial mock wrapping the live kernel. The previousArtisan::shouldReceivereplaced the whole console kernel with a full mock, so the test runner's own outerKernel::callhad no matching expectation.ProjectModelTest/PublicProjectWorkspacePropsTestaligned with owner-first role resolution:userProjectRole()returns'owner'for the project owner before consulting membership pivots, so the obsoleteuserWithEmailmock expectation and the'creator'assertion were stale.Security fixes
shell-quoteto>=1.8.4(GHSA-w7jw-789q-3m8p, critical, viaconcurrently).instantsearch.js→qsoverride to6.15.2(GHSA-q8mj-m7cp-5q26).vite≤6.4.1 viavitepress, no fix available) and do not trip the--audit-level=highgate.Test plan
php artisan test— full suite green locally: 2266 passed, 24 skipped, 0 failuresnpm audit --audit-level=highexits cleannpm run buildsucceeds with the bumped dependenciesvendor/bin/pint --dirtypasses