-
Notifications
You must be signed in to change notification settings - Fork 5
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
mops watch #254
mops watch #254
Changes from 17 commits
97aeb43
63a5628
27184b6
ee2252d
20dfb99
1761cfa
713db3f
5842ec5
80f0757
63c34ba
7300fda
e44e856
a4c45cd
64821d3
d3a27a0
7fd4974
566e00c
8a2e9bf
5e34b3c
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 |
---|---|---|
|
@@ -5,14 +5,25 @@ import {Reporter} from './reporter.js'; | |
import {TestMode} from '../../../types.js'; | ||
|
||
export class SilentReporter implements Reporter { | ||
total = 0; | ||
passed = 0; | ||
failed = 0; | ||
skipped = 0; | ||
passedFiles = 0; | ||
failedFiles = 0; | ||
passedNamesFlat : string[] = []; | ||
flushOnError = true; | ||
errorOutput = ''; | ||
onProgress = () => {}; | ||
Comment on lines
+15
to
+17
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. 🛠️ Refactor suggestion Refactor to Avoid Redundant Default Values The properties Suggested Refactor: Option 1—Remove default values from property declarations: - flushOnError = true;
- errorOutput = '';
- onProgress = () => {};
constructor(flushOnError = true, onProgress = () => {}) {
this.flushOnError = flushOnError;
this.onProgress = onProgress;
+ this.errorOutput = '';
} Option 2—Keep defaults in property declarations and adjust constructor parameters: constructor(flushOnError?: boolean, onProgress?: () => void) {
if (flushOnError !== undefined) {
this.flushOnError = flushOnError;
}
if (onProgress !== undefined) {
this.onProgress = onProgress;
}
} Also applies to: 19-22 |
||
|
||
addFiles(_files : string[]) {} | ||
constructor(flushOnError = true, onProgress = () => {}) { | ||
this.flushOnError = flushOnError; | ||
this.onProgress = onProgress; | ||
} | ||
|
||
addFiles(files : string[]) { | ||
this.total = files.length; | ||
} | ||
|
||
addRun(file : string, mmf : MMF1, state : Promise<void>, _mode : TestMode) { | ||
state.then(() => { | ||
|
@@ -30,10 +41,17 @@ export class SilentReporter implements Reporter { | |
this.failedFiles += Number(mmf.failed !== 0); | ||
|
||
if (mmf.failed) { | ||
console.log(chalk.red('✖'), absToRel(file)); | ||
mmf.flush('fail'); | ||
console.log('-'.repeat(50)); | ||
let output = `${chalk.red('✖')} ${absToRel(file)}\n${mmf.getErrorMessages().join('\n')}\n${'-'.repeat(50)}`; | ||
|
||
if (this.flushOnError) { | ||
console.log(output); | ||
} | ||
else { | ||
this.errorOutput = `${this.errorOutput}\n${output}`.trim(); | ||
} | ||
Comment on lines
+46
to
+51
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. 🛠️ Refactor suggestion Refactor Error Output Accumulation for Efficiency Accumulating error messages using string concatenation may become inefficient with a large number of errors. Consider using an array to collect error messages and join them when needed. Apply this refactor to improve performance and maintainability: - else {
- this.errorOutput = `${this.errorOutput}\n${output}`.trim();
- }
+ else {
+ if (!this.errorOutputArray) {
+ this.errorOutputArray = [];
+ }
+ this.errorOutputArray.push(output);
+ } Then, when you need to access the accumulated error output: getErrorOutput() {
return this.errorOutputArray.join('\n');
} |
||
} | ||
|
||
this.onProgress(); | ||
}); | ||
} | ||
|
||
|
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.
Incorrect usage of
promisify
withspawn
functionThe
child_process.spawn
function does not follow the standard Node.js callback style and returns aChildProcess
instance, not a function that accepts a callback. Therefore, usingpromisify
onspawn
will not work as expected. This issue affects the import at line 6 and the usages at lines 148-154 and 159-165. Instead, consider creating a custom Promise-based wrapper aroundspawn
to handle asynchronous execution and error handling properly.To fix this, remove the import of
promisify
and implement a custom helper function:Also applies to: 148-154, 159-165