Skip to content
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

ref(trace) refactor children representation #78664

Open
wants to merge 1 commit into
base: jb/ref/tracetree
Choose a base branch
from

Conversation

JonasBa
Copy link
Member

@JonasBa JonasBa commented Oct 5, 2024

It was super annoying to have to deal with both spanChildren and children from the pov of the tree model. This has bothered me as it made a lot of things annoying wrt how the tree was actually constructed. The second part was that the children getter was causing a lot of errors and made it annoying to track what children we are actually rendering.

From the pov of testing, the new tree model is serializable, which means we can just use snapshot testing to assert the tree is the shape that we expect it to be. The diff is partially large as I renamed the old test file which had 3k loc tests. I plan on porting over those tests to simpler snapshot tests gradually in the coming weeks.

@JonasBa JonasBa requested a review from Abdkhan14 October 5, 2024 19:02
@JonasBa JonasBa requested a review from a team as a code owner October 5, 2024 19:02
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 5, 2024
Copy link

codecov bot commented Oct 5, 2024

Bundle Report

Changes will increase total bundle size by 107.72kB (0.35%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
app-webpack-bundle-array-push 30.77MB 107.72kB (0.35%) ⬆️

Copy link

codecov bot commented Oct 5, 2024

❌ 29 Tests Failed:

Tests completed Failed Passed Skipped
7963 29 7934 0
View the top 3 failed tests by shortest run time
MissingInstrumentationNode shouldInsertMissingInstrumentationSpan false if next is not a span MissingInstrumentationNode shouldInsertMissingInstrumentationSpan false if next is not a span
Stack Traces | 0.002s run time
TypeError: (0 , _missingInstrumentationNode.shouldInsertMissingInstrumentationSpan) is not a function
    at Object.<anonymous> (.../newTraceDetails/traceModels/missingInstrumentationNode.spec.tsx:27:85)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
MissingInstrumentationNode shouldInsertMissingInstrumentationSpan true if gap is sufficiently large MissingInstrumentationNode shouldInsertMissingInstrumentationSpan true if gap is sufficiently large
Stack Traces | 0.003s run time
TypeError: (0 , _missingInstrumentationNode.shouldInsertMissingInstrumentationSpan) is not a function
    at Object.<anonymous> (.../newTraceDetails/traceModels/missingInstrumentationNode.spec.tsx:49:85)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
VirtualizedViewManger expandToPath error handling scrolls to child span of parent autogrouped node when path is missing autogrouped node VirtualizedViewManger expandToPath error handling scrolls to child span of parent autogrouped node when path is missing autogrouped node
Stack Traces | 0.003s run time
TypeError: Cannot read properties of null (reading 'meta')
    at visit (.../newTraceDetails/traceModels/traceTree.tsx:18199:46)
    at traceQueueIterator (.../newTraceDetails/traceModels/traceTree.tsx:22265:9)
    at Function.FromTrace (.../newTraceDetails/traceModels/traceTree.tsx:18255:5)
    at makeSingleTransactionTree (.../newTraceDetails/traceRenderers/virtualizedViewManager.spec.tsx:89:31)
    at Object.<anonymous> (.../newTraceDetails/traceRenderers/virtualizedViewManager.spec.tsx:518:22)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant