[#12588] Improve test code coverage of LogsDetails #12618
Conversation
|
hi @nikkixiong, do fix the lint issues before we proceed to review this PR, you can check if there are lint issues locally by running |
|
hi @cedricongjh , I already fixed the lint issues and ready to be reviewed. |
| expect(component.details).toEqual(log.details); | ||
| }); | ||
|
|
||
| // Test for Branch |
There was a problem hiding this comment.
nit: lets remove these comments (before each it and describe), the strings are sufficient in describing what the test is for
| it('should not set details when log details event is not EMAIL_SENT', () => { | ||
| const log: GeneralLogEntry = { | ||
| severity: LogSeverity.DEFAULT, | ||
| trace: 'test_trace', | ||
| insertId: '1', | ||
| resourceIdentifier: { id: '2' }, | ||
| sourceLocation: { file: 'log.txt', line: 42, function: 'test_function' }, | ||
| timestamp: Date.now(), | ||
| message: 'test_message', | ||
| details: { | ||
| event: LogEvent.EXCEPTION_LOG, | ||
| message: 'email_sent', | ||
| }, | ||
| }; | ||
|
|
||
| component.log = log; | ||
|
|
||
| expect(component.logValue).toEqual(log); | ||
| expect(component.details).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
let's also add a test for checking these lines:
this.emailContent = details.emailContent;
details.emailContent = undefined;
this.details = details;
check that if there's email content, its set to undefined in details, and that the this.emailContent contains the email content
| exceptionDetails.exceptionMessages = ['message']; | ||
|
|
||
| component.log = { ...log, details: exceptionDetails }; | ||
| expect(component.exceptionStackTrace).toContain(': message'); |
There was a problem hiding this comment.
it would be better here if we expect the exceptionStackTrace to equal to a whole string, to make it clear how the string should look like
src/web/app/components/logs-table/log-details/exception-log-details.component.spec.ts
Outdated
Show resolved
Hide resolved
src/web/app/components/logs-table/log-details/request-log-details.component.spec.ts
Show resolved
Hide resolved
| @@ -22,4 +22,105 @@ describe('RequestLogDetailsComponent', () => { | |||
| it('should create', () => { | |||
| expect(component).toBeTruthy(); | |||
| }); | |||
|
|
|||
| it('should set log and details when input is provided', () => { | |||
There was a problem hiding this comment.
let's add a test for when details.requestBody is truthy and check for requestBody as well as details.requestBody
|
Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
…ils.component.spec.ts Co-authored-by: Cedric Ong <[email protected]>
…tails.component.spec.ts Co-authored-by: Cedric Ong <[email protected]>
|
Folks, This PR seems to be stalling (no activities for the past 8 days). 🐌 😢 |
|
Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢 |
|
Folks, This PR seems to be stalling (no activities for the past 11 days). 🐌 😢 |
|
Folks, This PR seems to be stalling (no activities for the past 9 days). 🐌 😢 |
|
hi @nikkixiong, wondering if you're still working on this PR? there are some comments that are still unaddressed |
|
Folks, This PR seems to be stalling (no activities for the past 13 days). 🐌 😢 |
|
Folks, This PR seems to be stalling (no activities for the past 16 days). 🐌 😢 |
|
hi @nikkixiong, we'll be closing this PR, do reopen it if you are still working on it |
Fixes #12588
Outline of Solution
Added tests to LogsDetails