-
Notifications
You must be signed in to change notification settings - Fork 9
Add ability to request list of installed packages #540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
fixes #535 This adds the `reportPackages` command to things that can be sent to the device agent, this will trigger on the next status check-in to add the list of packages and versions to the status message, along with if the `module_cache` is being used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid future breakage, it would be good if getState was checked to ensure new data in state object is present (and is correct/expected shape) when requested (and not present in state when not requested)?
See unit tests in
device-agent/test/unit/lib/agent_spec.js
Lines 493 to 494 in d7c0d87
| describe('getState', function () { | |
| it('returns partial state', async function () { |
lib/agent.js
Outdated
| const { modules } = this.launcher.readPackage() | ||
| if (!this.config.moduleCache) { | ||
| state.moduleCache = false | ||
| } else { | ||
| state.moduleCache = true | ||
| } | ||
| const realList = {} | ||
| const modulesList = Object.keys(modules) | ||
| for (const mod of modulesList) { | ||
| try { | ||
| const modPath = path.join(this.launcher.projectDir, 'node_modules', mod, 'package.json') | ||
| const content = readFileSync(modPath) | ||
| const packJSON = JSON.parse(content) | ||
| realList[mod] = packJSON.version | ||
| } catch (err) { | ||
| // bad package info | ||
| debug(`Project reading version of project ${mod}`) | ||
| } | ||
| } | ||
| state.packageList = realList | ||
| this.mqttClient.reportPackages = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick - If this were a function of the launcher it could be unit tested in isolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but the tests will be ugly (need to install a bunch of packages both with and without module_cache)
Steve-Mcl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ben, I dont see where readPackage flag is reset.
i.e. it looks like once set, it will report packages on every call to getState
A test to prove this might be:
agent_spec.js
it('return nodeRedVersion when reportPackages is true', async function () {
const agent = createMQTTAgent()
agent.launcher = Launcher.newLauncher()
agent.launcher.reportPackages = true
await agent.start()
// stub launcher readPackage (readPackage is a sync function) to return some packages
agent.launcher.readPackage = sinon.stub().returns({
modules: {
'node-red': '5.0.0'
}
})
const stateWithPackages = agent.getState()
stateWithPackages.should.have.property('nodeRedVersion', '5.0.0')
})
it('Ensure reportPackages is reset after reporting', async function () {
const agent = createMQTTAgent()
agent.launcher = Launcher.newLauncher()
agent.launcher.reportPackages = true
await agent.start()
agent.getState()
agent.launcher.reportPackages.should.be.false()
})reportPackages Tests
For unit tests, to avoid regression, can we test the reportPackages now that is is a launcher function?
To do this, we would just need to change the import of readFileSync in launcher so it can be stubbed (no need to do npm installs)
launcher.js
Line 4: const fsSync = require('fs')
Line 766: const content = fsSync.readFileSync(modPath)
launcher_spec.js
describe('reportPackages', function () {
it('Scans packages and reads versions', async function () {
const launcher = newLauncher({ config, checkIn: () => {} }, null, 'projectId', setup.snapshot)
should(launcher).be.an.Object()
await launcher.writeFlow()
await launcher.writeCredentials()
// stub installDependencies so we don't actually install anything when starting
sinon.stub(launcher, 'installDependencies').resolves()
// stub fsSync.readFileSync to return a package.json with known packages
sinon.replace(fsSync, 'readFileSync', sinon.fake((filePath) => {
const posixPath = filePath
.replace(/^([a-zA-Z]):/, '/$1') // Turn 'C:' into '/C'
.replace(/\\/g, '/') // Swap slashes
// fake the main project package.json
if (posixPath.endsWith('project/package.json')) {
return JSON.stringify({
name: 'TEST_PROJECT',
version: '0.0.0-aaaabbbbcccc',
description: 'A FlowForge managed Node-RED project',
dependencies: {
'node-red-node-random': '0.4.0'
}
})
}
// fake the node-red-node-random package.json
// should end with project/node_modules/node-red-node-random/package.json'
if (posixPath.endsWith('/no23de-red-node-random/package.json')) {
return JSON.stringify({
name: 'node-red-node-random',
version: '1.0.0',
description: 'node-red-node-random node',
dependencies: {
'random-dep': '0.4.0'
}
})
}
// default to original method for other files
return fsSync.readFileSync.wrappedMethod.apply(this, arguments)
}))
const report = launcher.reportPackages()
report.should.be.an.Object()
report.should.have.property('packageList')
report.packageList.should.be.an.Object()
report.packageList.should.have.property('node-red-node-random', '1.0.0')
})
})
It's reset on line 316 here: Lines 312 to 317 in d7f7c09
|
fixes #535
Description
This adds the
reportPackagescommand to things that can be sent to the device agent, this will trigger on the next status check-in to add the list of packages and versions to the status message, along with if themodule_cacheis being used.Related Issue(s)
#535
Checklist
flowforge.yml?FlowFuse/helmto update ConfigMap TemplateFlowFuse/CloudProjectto update values for Staging/ProductionLabels
area:migrationlabel