-
Notifications
You must be signed in to change notification settings - Fork 27
W-19638348: DO NOT MERGE Prerelease/agent testing #581
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
base: main
Are you sure you want to change the base?
Conversation
*/ | ||
|
||
private formatAgentTestResult( | ||
agentTestResults: ApexTestResult[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure agentTestResults
should be typed as AgentTestResult
based on the comment?
I'm confused why we're forcing agent->apex results... there's not a lot of overlap, and if the library can't handle that, we need to fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a schema perspective, agent and apex test results are both ApexTestResult records. However, not all fields are populated consistently. For example, agent test results do not have the apexClass field populated, which requires a normalization step. This is necessary to prevent null pointers in downstream methods that expect complete ApexTestResult records.
flowTestIds: records | ||
.filter((r) => r.ApexClassId === null) | ||
.map((r) => r.Id) | ||
apexTestResults, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we keep flow test items as IDs rather than results like Apex/Agent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ApexTestResult has the information about apex,agent and flow test. For agent and apex test it contains all information needed but for flow tests it doesn't have enough information so we need to query a second object called FlowTestResult.
Here we keep the agent and apex test records because that' all we need. For flow tests we need the Ids to use them in getFlowTestResults method to query FlowTestResult.
src/tests/types.ts
Outdated
*/ | ||
TestTimestamp: string; | ||
/** | ||
* Indicates the type or test. Undefines(Apex), flowTesting(Flow) or agentTesting(Agent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ApexTesting(Apex)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the posible values that come from the server. For apex tests the value will be undefined.
src/tests/testService.ts
Outdated
if (isFlow) { | ||
namespaceInfos = await this.processFlowTest( | ||
if (isFlowOrAgent) { | ||
namespaceInfos = await this.processFlowOrAgentTest( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make sense to processes these as
- isFlow
- isAgent
- else (isApex)
why combine flow and agent tests, let's keep the methods specific to their types of testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The testName for agent and flow tests have a prefix (AgentTesting and FlowTesting). This means the logic to extract the namespace from the testName is different than for apex test.
Agent and Flow test: prefix.namespace.testName
Apex: namespace.testName
The logic to extract the extract the namespace for agent and flow test is the same, that's why I'm calling the same method.
I'll rename the method and add a description to make it clearer.
return TestCategory.Flow; | ||
case TestNamespace.Agent: | ||
return TestCategory.Agent; | ||
default: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should be careful about defaulting to one type or the other, we'll end up with hard-to-diagnose server-side errors if we send a malformed request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a clarification: this method is not for building the request payload. Its purpose is to process the server's response.
I'm open to suggestions on improving its reliability.
test/tests/asyncTests.test.ts
Outdated
const result = await asyncTestSrv.getAsyncTestResults(testQueueItems); | ||
|
||
await asyncTestSrv.getAsyncTestResults(testQueueItems); | ||
console.log(mockToolingQuery.calledTwice); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: remove console
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
What does this PR do?
Adds support for Agentforce agent tests.
What issues does this PR fix or reference?
@W-19638348@