Skip to content

Conversation

@WillieRuemmele
Copy link
Contributor

What does this PR do?

checks the server response for other issues not present in the getFileResponse() method from SDR and displays them alongside other deployment issues.

What issues does this PR fix or reference?

@W-9816657@ forcedotcom/cli#1180

OLD

 ➜  sfdx force:source:deploy -m CustomField:Account.Name 
*** Deploying with SOAP API ***
Deploy ID: 0AfJ000001m2PoWKAU
SOURCE PROGRESS | ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ | 0/1 Components
ERROR running force:source:deploy:  Deploy failed.

NEW

 ➜  sfdx force:source:deploy -m CustomField:Account.Name 
*** Deploying with SOAP API ***
Deploy ID: 0AfJ000001m1diRKAQ
SOURCE PROGRESS | ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ | 0/1 Components

=== Component Failures [1]
Type   Name             Problem
─────  ───────────────  ──────────────────
Error  Account.NameNew  Not in package.xml

@WillieRuemmele WillieRuemmele requested review from a team, mshanemc and shetzel September 20, 2021 21:13
expect(tableStub.calledTwice).to.equal(true);
expect(styledHeaderStub.firstCall.args[0]).to.contain('Test Failures [1]');
expect(styledHeaderStub.secondCall.args[0]).to.contain('Apex Code Coverage');
expect(styledHeaderStub.calledThrice).to.equal(true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these tests were giving false positives and needed to be updated

}

if (this.result?.response?.details?.componentFailures) {
const deployMessages = toArray(this.result.response.details.componentFailures);
Copy link
Contributor

Choose a reason for hiding this comment

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

failures.push(...fileResponses);
}

if (this.result?.response?.details?.componentFailures) {
Copy link
Contributor

Choose a reason for hiding this comment

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

aren't componentFailures and fileResponses going to include some of the same errors, and so this would show duplicates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ugh. dupes 🤦

@mshanemc
Copy link
Contributor

if the dupes aren't happening, looks good.

const deployMessages = toArray(this.result?.response?.details?.componentFailures);
if (deployMessages.length > failures.length) {
// if there's additional failures in the API response, find the failure and add it to the output
deployMessages.map((deployMessage) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

this could get really loopy on a big deployment fail (use of find within a loop where find is doing multiple string.includes)

perf suggestion: iterate failures once to build a keyed Map< type+fullName>, deployMessage>
then, you can iterate the deployMessages once and use deployMessageMap.has( type + fullName)
No would be a good sign to add it
has=true could still compare the messages (if that's actually a source of dupes)

At the very least, do the (do type and name match?) conditional first so that it mostly exits before having to do the string.includes ops

if (this.result?.response?.details?.componentFailures) {
const deployMessages = toArray(this.result.response.details.componentFailures);
const deployMessages = toArray(this.result?.response?.details?.componentFailures);
if (deployMessages.length > failures.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

good idea

@mshanemc mshanemc merged commit 99df1ca into main Sep 22, 2021
@mshanemc mshanemc deleted the wr/findServerErrors branch September 22, 2021 16: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.

3 participants