DEVPROD-5130: Case on renderingType during log ingestion, introduce LOCAL_UPLOAD logType and fix bug related to exposing renderingType from hook #509
Conversation
Passing run #5353 ↗︎
Details:
Review all test suite changes for PR #509 ↗︎ |
|||||||||||||||
SupaJoon
changed the title
SupaJoon
changed the title
| @@ -9,10 +9,9 @@ import ResmokeRow from "../ResmokeRow"; | |||
|
|
|||
| type RowRendererFunction = (props: { | |||
| processedLogLines: ProcessedLogLines; | |||
| logType: LogTypes; | |||
There was a problem hiding this comment.
I changed this code to access logType from LogContext.logMetadata instead of a prop since the function already accesses the context. I'm open to making this change in a separate PR.
| @@ -33,6 +38,7 @@ import { getNextPage } from "./utils"; | |||
|
|
|||
| interface LogContextState { | |||
| expandedLines: ExpandedLines; | |||
| isUploadedLog: boolean; | |||
There was a problem hiding this comment.
This value used to be set directly via setLogMetadata when the user uploads a log. I now represent "uploaded log" with LogType.LOCAL_UPLOAD
| import { useLogContext } from "context/LogContext"; | ||
| import FileDropper from "./LogDrop/FileDropper"; | ||
|
|
||
| const LogDrop = () => { | ||
| const { hasLogs, logMetadata } = useLogContext(); |
There was a problem hiding this comment.
logMetadata is set during the log parsing steps of FileDropper and hasLogs is set to true after successful parsing and ingestion. This makes it appropriate for LogWindow to access logMetadata directly via context.
| renderingType === LogRenderingTypes.Resmoke | ||
| ? LogTypes.RESMOKE_LOGS | ||
| : LogTypes.EVERGREEN_TASK_LOGS; | ||
| const logType = LogTypes.LOCAL_UPLOAD; |
There was a problem hiding this comment.
I adjusted the code so LogTypes directly represents log origin and LogRenderingType represents how it should be rendered.
| ) : ( | ||
| <LogWindow logType={logType} /> | ||
| ); | ||
| return hasLogs === null ? <LoadingPage logType={logType} /> : <LogWindow />; |
There was a problem hiding this comment.
LoadingPage needs logType to save it into logMetadata. By the time LogWindow is rendered, that value is available via context.
| @@ -458,17 +458,16 @@ functions: | |||
| content_type: text/plain | |||
| permissions: public-read | |||
|
|
|||
| seed-logkeeper: | |||
| seed-bucket-data: | |||
There was a problem hiding this comment.
Update the name to help phase out Logkeeper.
| echo "${YELLOW}LK_CORS_ORIGINS=http:\/\/localhost:\\\d+ LK_EVERGREEN_ORIGIN=http://localhost:8080 LK_PARSLEY_ORIGIN=http://localhost:5173 go run main/logkeeper.go --localPath $PWD/bin/_bucketdata${NC}" | ||
|
|
||
| echo "Create symlink in your local evergreen directory:" | ||
| echo "ln -s $PWD/bin/_bucketdata _bucketdata" |
There was a problem hiding this comment.
EVG expects bucketdata to exist in the EVG directory and Logkeeper will expect it to exist at the dir defined in --localPath
| const { setWrap } = preferences; | ||
|
|
||
| useEffect(() => { | ||
| ingestLines(ansiLogLines, LogTypes.EVERGREEN_TASK_LOGS); | ||
| setLogMetadata({ logType: LogTypes.EVERGREEN_TASK_LOGS }); |
There was a problem hiding this comment.
I updated some components to access logType from context instead of props after data is set in the "loading" phase of the app. This decreases data management complexity. Props and context data shouldn't be used interchangeably because they can diverge.
| buildID, | ||
| execution, | ||
| logType, | ||
| taskID, | ||
| }); | ||
|
|
||
| if (loading || !task) { | ||
| const { data: testData, loading: isLoadingTest } = useQuery< |
There was a problem hiding this comment.
Breadcrumb now shows the test name in the trailing breadcrumb for EVG tests.
| buildID, | ||
| execution, | ||
| fileName, | ||
| isUploadedLog, |
There was a problem hiding this comment.
Pulled this value out of logMetadata because it maps on logMetadata.logType.
| @@ -204,7 +204,11 @@ export const useResolveLogURLAndRenderingType = ({ | |||
| downloadURL = rawLogURL; | |||
| if (!renderingTypeFromQuery) { | |||
| renderingType = LogRenderingTypes.Default; | |||
| } else if (renderingTypeFromQuery in LogRenderingTypes) { | |||
There was a problem hiding this comment.
This works only on non-const, number-based enums
|
Ported this PR to evergreen-ci/ui |
DEVPROD-5130
Description
These code changes:
resmokerendering type when applicableTesting
I unit tested the
useResolveLogURLAndRenderingTypehook and then asserted the correct rendering logic is utilized in E2E tests. It's probably useful to ensure ingestLogs is called with the correct rendering type in context which means we would have to unit test LoadingPage. We should actually do this eventually because LoadingPage makes guarantees about the state of context during the log loading phase of the app (DEVPROD-5891).