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

fix: avoid unknown error #652

Merged
merged 13 commits into from
Jun 8, 2023
Merged

Conversation

mshanemc
Copy link
Contributor

@mshanemc mshanemc commented Jun 6, 2023

NUTs require forcedotcom/source-tracking#412

What does this PR do?

gives better output when nothing is specified (ie pull) and there's nothing to retrieve
adds a NUT for exit code on empty "pull-like" retrieves
add warning for retrieve operations that came back empty

What issues does this PR fix or reference?

forcedotcom/cli#2126 @W-13329259@

QA notes

ui reminder: bin/dev has different styling and more verbose warning output than a linked plugin.
sf-plugin-core ux doesn't seem to style the warning the same way sfCommand does, but that's out of scope for this WI

fileResponsesFromDelete.length === 0 &&
(flags.manifest || flags.metadata || flags['source-dir'])
) {
throw new SfError('No metadata retrieved');
Copy link
Contributor

Choose a reason for hiding this comment

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

worth a unit test or nut?

Copy link
Contributor

Choose a reason for hiding this comment

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

should that even be an error? I think we could print that message and exit 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Steve, yes, will do to go with the STL PR.
Willie...trying to make it match how deploy works (retrieve "changes" can be empty, but retrieves with specified material should not). ref: forcedotcom/cli#2065

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FyI, the retrieve will retrieve an "empty" package.xml so even when nothing retrieves, it still looks like a success. We'd have to do more work to make it NOT do that (ex: check that there's anything in there besides package.xml) so I'm inclined to leave it as a success (the formatter will give a warning since the user probably didn't intend that).

The "there was nothing in the CS for a pull-like retrieve" output also moved to the formatter. It seems to work well.

There's also a NUT that helps make sure that an empty pull-like retrieve is a exit0.

await testkit.retrieve({ args: '--metadata ApexClass' });
await testkit.expect.filesToBeRetrieved(['force-app/main/default/classes/*']);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you removing this? Is it covered elsewhere?

@@ -133,6 +133,13 @@ describe('end-to-end-test for tracking with an org (single packageDir)', () => {
);
});

it('an empty "pull" retrieve is not an error', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this assertion moved to forceIgnore.nut and I forgot to remove it from basics.

[explanation: there's a profile change here. I had to ignore profiles over in the forceIgnore.nut to ensure an empty "pull"

@shetzel shetzel merged commit f5d5cb7 into main Jun 8, 2023
@shetzel shetzel deleted the sm/unknown-error-for-empty-retrieves branch June 8, 2023 19:15
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