ID-158: Frontend dependency updates#789
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #789 +/- ##
==========================================
+ Coverage 84.93% 89.79% +4.85%
==========================================
Files 298 291 -7
Lines 14097 8365 -5732
Branches 1992 2004 +12
==========================================
- Hits 11973 7511 -4462
+ Misses 2116 799 -1317
- Partials 8 55 +47
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:
|
| # The lockfile may be committed from macOS, which omits Linux-specific optional | ||
| # packages (e.g. platform-native bindings). Regenerating it here adds those | ||
| # entries so the subsequent `npm ci` can resolve them on this Linux runner. | ||
| - run: npm install --package-lock-only --ignore-scripts |
There was a problem hiding this comment.
The workaround (npm install --package-lock-only before npm ci) has two problems:
The npm cache key is hashFiles(**/package-lock.json) on the committed lockfile. After regeneration the lock file has changed, so npm ci installs against a different lockfile than what was cached. The cache hit is partial at best.
npm ci's whole purpose is reproducible installs from a committed lockfile. Mutating the lockfile first undermines that guarantee — any dependency drift introduced by the regeneration won't be caught.
Better fix: Regenerate the lockfile once from a Linux runner and commit it. All three CI jobs have the same comment/workaround; if the root cause (macOS-committed lockfile) is addressed, all three go away.
There was a problem hiding this comment.
generated from a linux system and removed these lines. Add a note in the README on the recommended way to make these updates.
| expect(screen.getByRole('combobox')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('selects a preset', async () => { |
There was a problem hiding this comment.
The test only checks that the dropdown label changes. It does not assert that applyPreset was called with the right session ID and preset ID. Since the confirmation modal was removed and the preset is now applied immediately, this is the most important behavioral path — yet it's untested. The real applyPreset is called, hits jsdom's fetch (which rejects), the error is silently swallowed by the .catch() snackbar handler, and the test still passes. The other new tests (sorts presets...) correctly mock applyPreset, so the pattern is there.
There was a problem hiding this comment.
updated the test to verify that the expected calls occur after selection
| ## Running Inferno Core for Development Purposes | ||
| ``` | ||
| # Install dependencies | ||
| npm install |
There was a problem hiding this comment.
should we still be running npm install?
There was a problem hiding this comment.
Yes, because you need to run it to get npm packages loaded onto your local system. But updates to package-lock.json shouldn't be committed. I think the note in the README around updating npm packages covers this.
2 participants
Summary
This PR updates the frontend dependencies and cleans up the associated tests, including
Testing Guidance
Run the demo suite and interact with the UI to make sure that everything still works as expected.