Repository navigation
fix(rig-bridge): tests no longer overwrite the real rig-bridge config - #1271
Merged
Merged
Conversation
The brightness and APRS-TNC plugins saved settings with
require('../core/config').saveConfig(), which always writes the core config
object to ~/.config/openhamclock/rig-bridge-config.json. A test that created
the brightness plugin with its own config and changed a setting therefore
overwrote the user's real file with the defaults (radio none, empty token).
- brightness and aprs-tnc use the saveConfig the core passes to plugins
(services.saveConfig), like gpio already does, and skip saving without it.
- core/config.js: RIG_BRIDGE_CONFIG_DIR overrides the config location
outright (no legacy lookup or migration).
- src/test/setup.js points RIG_BRIDGE_CONFIG_DIR at a temp dir, so no test
can reach the real file.
- Tests for the injected saves and the test config location.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
accius
approved these changes
Nov 9, 2026
accius
left a comment
Owner
There was a problem hiding this comment.
Thanks, Koen. Good diagnosis: require('../core/config').saveConfig() always writes the core config object, so any plugin created with its own config overwrote the real file. Using the saveConfig the core already passes to plugins (rig-bridge.js line 86), and skipping the save without it, is the right fix. The RIG_BRIDGE_CONFIG_DIR setup guard is a nice extra layer.
I verified it on a machine that had been bitten. Test-merged onto current Staging, I ran the full suite including hardware-plugins.test.js (310 files, 3,775 tests pass). The real ~/.config/openhamclock/rig-bridge-config.json had the same modification time and SHA-1 before and after. Approving.
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.
Problem
Running the test suite (
npx vitest run) overwrote the developer's real~/.config/openhamclock/rig-bridge-config.jsonwith defaults: radionone, empty API token, serial port gone.The cause is that
plugins/brightness.jsandplugins/aprs-tnc.jssaved settings withrequire('../core/config').saveConfig(). That always writes the core config object to the real config path, whatever config the plugin was created with. The brightness tests inhardware-plugins.test.jscreate the plugin with their own config and change a setting, so each run replaced the user's file with the core defaults.Fix
brightnessandaprs-tncnow use thesaveConfigthe core already passes to every plugin (services.saveConfig,rig-bridge.js), likegpiodoes. Without it they skip saving instead of falling back to the real file. Production behaviour is unchanged.core/config.js: a newRIG_BRIDGE_CONFIG_DIRenvironment variable overrides the config location outright, with no legacy lookup or migration.src/test/setup.js: setsRIG_BRIDGE_CONFIG_DIRto a temp dir for every test run, so even a future test that saves config can't reach the real file.Tests
saveConfigonly when a setting changes, and works without it.setBeaconEnabledwrites through to the config and saves through the injectedsaveConfig, and doesn't throw without it.core/config.test.js: under test,CONFIG_DIR/CONFIG_PATHpoint at the temp dir, not~/.config/openhamclock.Verified
Full suite run with a scratch
HOME: 310 files and 3,776 tests passed. Nothing was written to the scratchHOME, where the brightness test used to write. The real config file's checksum and modification time were unchanged. eslint and prettier are clean.🤖 Generated with Claude Code