-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: Enhance cli command line #107
Changes from all commits
9022da9
726a228
f00a08e
cb171c7
7d62465
998ca5e
f290505
aa288b9
3e5a7ea
9d6d134
aecc925
cadabd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,57 +1,69 @@ | ||||||
let path = require('path') | ||||||
let exec = require('node:child_process').exec | ||||||
|
||||||
const serverOpts = { | ||||||
port: 0, | ||||||
host: '127.0.0.1', | ||||||
} | ||||||
|
||||||
describe('cli', () => { | ||||||
describe('without arguments', () => { | ||||||
test('should show help without required url', async () => { | ||||||
const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {}) | ||||||
const mockConsoleError = jest | ||||||
.spyOn(console, 'error') | ||||||
.mockImplementation(() => {}) | ||||||
|
||||||
expect(() => { | ||||||
require('../cli.js') | ||||||
}).toThrow('Missing URL') | ||||||
|
||||||
expect(mockExit).toHaveBeenCalledWith(1) | ||||||
}) | ||||||
const cliExec = (args) => { | ||||||
const command = `${path.resolve(`${__dirname}/../cli.js`)} ${args.join(' ')}` | ||||||
return new Promise(resolve => { | ||||||
exec(command, | ||||||
Check warning Code scanning / CodeQL Shell command built from environment values
This shell command depends on an uncontrolled [absolute path](1).
This shell command depends on an uncontrolled [absolute path](2).
|
||||||
// Setting exit override to allow program to exit simplifying unit testing | ||||||
{ env: { ...process.env, 'EXIT_OVERRIDE': 'true' } }, | ||||||
(error, stdout, stderr) => { | ||||||
resolve({ | ||||||
code: error && error.code ? error.code : 0, | ||||||
error, | ||||||
stdout, | ||||||
stderr | ||||||
}) | ||||||
}) | ||||||
}) | ||||||
} | ||||||
|
||||||
describe('with arguments', () => { | ||||||
let gbfsFeedServer | ||||||
describe('cli', () => { | ||||||
|
||||||
beforeAll(async () => { | ||||||
gbfsFeedServer = require('./fixtures/server')() | ||||||
let gbfsFeedServer | ||||||
let feedUrl | ||||||
|
||||||
await gbfsFeedServer.listen(serverOpts) | ||||||
beforeAll(async () => { | ||||||
gbfsFeedServer = require('./fixtures/server')() | ||||||
await gbfsFeedServer.listen(serverOpts) | ||||||
feedUrl = `http://${gbfsFeedServer.server.address().address}:${gbfsFeedServer.server.address().port}/gbfs.json` | ||||||
}) | ||||||
|
||||||
return gbfsFeedServer | ||||||
}) | ||||||
afterAll(() => { | ||||||
return gbfsFeedServer.close() | ||||||
}) | ||||||
|
||||||
afterAll(() => { | ||||||
return gbfsFeedServer.close() | ||||||
}) | ||||||
test('should show an error if url parameter is not set', async () => { | ||||||
const result = await cliExec([]) | ||||||
expect(result.code).toBe(1) | ||||||
expect(result.error.message).toContain('error: required option \'-u, --url <feed_url>\' not specified') | ||||||
}) | ||||||
|
||||||
test('should run cli', async () => { | ||||||
const url = `http://${gbfsFeedServer.server.address().address}:${ | ||||||
gbfsFeedServer.server.address().port | ||||||
}` | ||||||
test('should success and print the report when url parameter set and -pr is set as default', async () => { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Only if you accept the suggestion to use
Suggested change
|
||||||
const result = await cliExec([`-u`, `${feedUrl}`]) | ||||||
expect(result.code).toBe(0) | ||||||
expect(result.stdout).toContain('summary:') | ||||||
}) | ||||||
|
||||||
process.argv[2] = url | ||||||
test('should show an error if pr parameter is set to `no` and -s is not set', async () => { | ||||||
const result = await cliExec([`-u ${feedUrl}`, '-pr no']) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Only if you accept the suggestion to use
Suggested change
|
||||||
expect(result.code).toBe(1) | ||||||
expect(result.stdout).toContain('Please set at least one of the following options: --save-report or --print-report') | ||||||
}) | ||||||
|
||||||
const cli = await require('../cli.js') | ||||||
test('should success when paramters url and save report has valid values and print report is set to no', async () => { | ||||||
const result = await cliExec([`-u ${feedUrl}`, '-s /dev/null', '-pr no']) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Only if you accept the suggestion to use
Suggested change
|
||||||
expect(result.code).toBe(0) | ||||||
}) | ||||||
|
||||||
expect(cli).toMatchObject({ | ||||||
summary: expect.objectContaining({ | ||||||
version: { detected: '2.2', validated: '2.2' }, | ||||||
hasErrors: true, | ||||||
validatorVersion: '1.0.0', | ||||||
errorsCount: 1 | ||||||
}), | ||||||
files: expect.any(Array) | ||||||
}) | ||||||
}) | ||||||
test('should success and print report when paramters url and save report are valid and print report is set to yes', async () => { | ||||||
const result = await cliExec([`-u ${feedUrl}`, '-s /dev/null', '-pr yes']) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Only if you accept the suggestion to use
Suggested change
|
||||||
expect(result.code).toBe(0) | ||||||
expect(result.stdout).toContain('summary:') | ||||||
}) | ||||||
}) | ||||||
}) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,17 +1,95 @@ | ||||||
#!/usr/bin/env node | ||||||
|
||||||
const { inspect } = require('util') | ||||||
const GBFS = require('./') | ||||||
const commander = require('commander') | ||||||
const fs = require('fs') | ||||||
const fsPath = require('path') | ||||||
const GBFS = require('./gbfs') | ||||||
const pjson = require('./package.json') | ||||||
|
||||||
getFeedValidationReport = async (url) => { | ||||||
const gbfs = new GBFS(url) | ||||||
return gbfs.validation() | ||||||
} | ||||||
|
||||||
const isExitOverrided = () => (process.env.EXIT_OVERRIDE === 'true') | ||||||
|
||||||
if (!process.argv[2]) { | ||||||
console.error('Usage: gbfs-validator GBFS_URL') | ||||||
process.exit(1) | ||||||
const printingReport = (options) => (options.printReport === 'yes') | ||||||
|
||||||
const exitProcess = (code) => { | ||||||
if (!isExitOverrided && code === 1) { | ||||||
process.exit(code) | ||||||
} | ||||||
process.exit(0) | ||||||
} | ||||||
|
||||||
const gbfs = new GBFS(process.argv[2]) | ||||||
parseOptions = () => { | ||||||
commander | ||||||
.version(pjson.version, '-v, --version') | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's always There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is in preparation for publishing an npm package. In the case of the command line, if we would like to include the commit hash, more work is needed to make available the hash information on local computers. |
||||||
.usage('[OPTIONS]...') | ||||||
.requiredOption('-u, --url <feed_url>', 'URL of the GBFS feed') | ||||||
.option('-vb, --verbose', 'Verbose mode prints debugging console logs') | ||||||
.option('-s, --save-report <report_path>', 'Local path to output report file') | ||||||
.addOption(new commander.Option('-pr, --print-report <yes_no>', 'Print report to standard output').default('yes').choices(['yes', 'no'])) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I connected with @richfab, and we couldn't find any strong feelings either way. Let's keep it as it is for simplicity; in the future, more options will be added, and the short argument name will be always a point of conflict. |
||||||
|
||||||
// Supporting friendly unit testing and possible CI integrations | ||||||
// The process throw an exception on parsing error in addition to the parsing error | ||||||
if (isExitOverrided()) { | ||||||
commander.exitOverride() | ||||||
} | ||||||
return commander.parse(process.argv).opts() | ||||||
} | ||||||
|
||||||
const saveReport = (report, filePath) => { | ||||||
const dirname = fsPath.dirname(filePath); | ||||||
if (!fs.existsSync(dirname)) { | ||||||
fs.mkdirSync(dirname, { recursive: true }); | ||||||
} | ||||||
fs.writeFileSync(filePath, JSON.stringify(report)) | ||||||
} | ||||||
|
||||||
const processFeedValidation = async (options) => { | ||||||
if (options.verbose) { | ||||||
console.log("Started GBFS validation with options: \n " + inspect(options, { depth: null, colors: true })) | ||||||
} | ||||||
try { | ||||||
const report = await getFeedValidationReport(options.url) | ||||||
if (printingReport(options)) { | ||||||
console.log(inspect(report, { depth: null, colors: true })) | ||||||
} | ||||||
if (options.saveReport) { | ||||||
saveReport(report, options.saveReport) | ||||||
} | ||||||
} catch (error) { | ||||||
console.error(`Critical error while validating GBFS feed => ${error}`) | ||||||
exitProcess(1); | ||||||
} | ||||||
} | ||||||
|
||||||
const validate = () => { | ||||||
const options = parseOptions() | ||||||
if (!options.saveReport && !printingReport(options)) { | ||||||
console.log('Please set at least one of the following options: --save-report or --print-report') | ||||||
commander.help() | ||||||
exitProcess(1) | ||||||
} | ||||||
|
||||||
processFeedValidation(options).then( | ||||||
() => { | ||||||
if (options.verbose) { | ||||||
console.log("Validation completed") | ||||||
} | ||||||
} | ||||||
) | ||||||
} | ||||||
|
||||||
if (require.main === module) { | ||||||
gbfs | ||||||
.validation() | ||||||
.then(result => console.log(inspect(result, { depth: null, colors: true }))) | ||||||
validate() | ||||||
} else { | ||||||
module.exports = gbfs.validation() | ||||||
} | ||||||
module.exports = { | ||||||
validate, | ||||||
processFeedValidation, | ||||||
saveReport, | ||||||
getFeedValidationReport, | ||||||
} | ||||||
} |
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.
(Only if you accept the suggestion to use
-p
for consistency with-s
)