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

feat!: Modify apex-node TestResult to account for test setup data #387

Merged
merged 45 commits into from
Jul 16, 2024

Conversation

CristiCanizales
Copy link
Contributor

@CristiCanizales CristiCanizales commented Jun 27, 2024

What does this PR do?

The new API contract only requires changes to the summary section; addition of a new property, testSetupTimeInMs, and a change in meaning of testTotalTimeInMs. The major change is the new section “setup”, which is less likely to break consumers.

Modify TestResult as follows:

  • Add summary property named "testSetupTimeInMs" as a number
  • Add a new top level section named "setup" as an array of ApexTestSetupData

For each function that returns TestResult today and is producing test results in the form of TestResultRaw

  • Modify it so that it uses a common transform to produce TestResult from TestResultRaw

What issues does this PR fix or reference?

@W-15923420@

Functionality After

  • TestResult summary contains new field "testSetupTimeInMs"
  • TestResult summary field "testTotalTimeMs" = sum of testSetupTimeInMs and testExecutionTimeMs
  • Heap space is not negatively impacted
image

@CristiCanizales CristiCanizales self-assigned this Jun 27, 2024
@CristiCanizales CristiCanizales changed the title Modify apex-node TestResult to account for test setup dat Modify apex-node TestResult to account for test setup data Jun 27, 2024
@CristiCanizales CristiCanizales changed the title Modify apex-node TestResult to account for test setup data [WIP] - Modify apex-node TestResult to account for test setup data Jun 27, 2024
@CristiCanizales CristiCanizales marked this pull request as ready for review July 3, 2024 21:41
@CristiCanizales CristiCanizales changed the title [WIP] - Modify apex-node TestResult to account for test setup data \Modify apex-node TestResult to account for test setup data Jul 3, 2024
@CristiCanizales CristiCanizales changed the title \Modify apex-node TestResult to account for test setup data Modify apex-node TestResult to account for test setup data Jul 3, 2024
Copy link
Collaborator

@peternhale peternhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my commit to this branch.

src/tests/asyncTests.ts Outdated Show resolved Hide resolved
src/tests/asyncTests.ts Outdated Show resolved Hide resolved
src/streaming/streamingClient.ts Outdated Show resolved Hide resolved
test/streaming/streamingClient.test.ts Outdated Show resolved Hide resolved
test/testData.ts Show resolved Hide resolved
Copy link
Collaborator

@peternhale peternhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a few minor adjustments. looking great!

Copy link
Collaborator

@peternhale peternhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CristiCanizales thank you. One remaining item is to ensure that when the PR is merged it is doe with the feat!: commit tag, so it triggers a major version bump.

src/tests/asyncTests.ts Show resolved Hide resolved
@CristiCanizales CristiCanizales changed the title Modify apex-node TestResult to account for test setup data feat!: Modify apex-node TestResult to account for test setup data Jul 16, 2024
@CristiCanizales CristiCanizales merged commit abf50aa into main Jul 16, 2024
16 checks passed
@CristiCanizales CristiCanizales deleted the cristi/w-15923420 branch July 16, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants