Skip to content

test: additional test coverage#1705

Merged
lesya7 merged 4 commits intomainfrom
lt/additional-tests
Mar 11, 2026
Merged

test: additional test coverage#1705
lesya7 merged 4 commits intomainfrom
lt/additional-tests

Conversation

@lesya7
Copy link
Copy Markdown
Collaborator

@lesya7 lesya7 commented Mar 10, 2026

What does this PR do?

What issues does this PR fix or reference?

#, @W-21531714@

Functionality Before

Some files have < 80%

<insert gif and/or summary>

Functionality After

image

<insert gif and/or summary>

@lesya7 lesya7 requested a review from bpbuch March 10, 2026 21:39
lesya7 added 2 commits March 10, 2026 14:42
Made-with: Cursor
Resolves Connection type mismatch in downstream plugin-deploy-retrieve
CI where duplicate @salesforce/core copies caused TS2322 errors on the
usernameOrConnection property.

Made-with: Cursor
const deployId = asyncResult.jsonOutput!.result.id;

for (let attempt = 0; attempt < 40; attempt++) {
execSync('sleep 3');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: Is there a reason you're using execSync('sleep 3') instead of setTimeout(...., 3000) or setInterval(...., 3000)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I needed synchronous blocking inside a synchronous test function. As far as I know setTimeout is async and would require converting the helper and all callers to async/await. That said, execSync('sleep 3') is ugly, agree :). Happy to refactor deployAndWait to be async with a promise-based delay instead. Will update.

Made-with: Cursor
Comment on lines +44 to +46
const deployId = asyncResult.jsonOutput!.result.id;

for (let attempt = 0; attempt < 40; attempt++) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was thinking that you'd have the logic of the project deploy report call within the callback for setTimeout(), but honestly, for a NUT, this is probably fine too.

@lesya7 lesya7 merged commit 6527359 into main Mar 11, 2026
47 checks passed
@lesya7 lesya7 deleted the lt/additional-tests branch March 11, 2026 17:53
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.

4 participants