Adding functionality to output test results in .xml format #77
Conversation
nairmai
commented
Aug 26, 2019
nairmai
commented
- Created a new class (XmlWriterService.ts) to provide this functionality
- Wrote unit tests for this class (XmlWriterService.test.ts)
- Added some helper utilities and interfaces (Utils.ts and types.ts)
- Modified the options for the d-ser-t-cli flags and d-ser-t-service main.ts to handle xml output
There was a problem hiding this comment.
I'm not working on this close enough to feel comfortable approving, most of this looks good. Frankly not familiar with Typescript enough to know if interfaces are widely used, but I'd say that's the most important bit of my review.
| const outputFileType = path.extname(String(this.outFile)).substr(1); | ||
| console.log('Output file type: ' + outputFileType); | ||
|
|
||
| if (outputFileType === 'json') { |
There was a problem hiding this comment.
This block makes me think that we might want this.localFileService to be an interface that either jsonWriterService or xmlWriterService implements, each of which would have a writeToTextFile method. That way this if statement doesn't get too long.
That may be overkill and I'm not sure how many output file types we're really going to end up supporting.
There was a problem hiding this comment.
I can totally see needing to extend this in the future. I think for now we want a working version of this merged in as soon as we can so I'll create an issue for this.
| */ | ||
| public static zeroPrefix(arr: string[]): string[] { | ||
| arr.forEach((elem, i) => { | ||
| if (Number(elem) < 10) { |
There was a problem hiding this comment.
nit: this would freak out if we ever got something negative. I don't actually think we should fix it, if we get a negative here I think we want it to fail, but interested to hear what others think
There was a problem hiding this comment.
I wrote this with the only intention of using it to format timestamps neatly, which I assumed would never have a negative input, but I've added a check within that if statement to only prepend '0' if elem is within [0,10).
| const outputFileType = path.extname(String(this.outFile)).substr(1); | ||
| console.log('Output file type: ' + outputFileType); | ||
|
|
||
| if (outputFileType === 'json') { |
There was a problem hiding this comment.
I can totally see needing to extend this in the future. I think for now we want a working version of this merged in as soon as we can so I'll create an issue for this.
