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

updating error message and fixing retry snapshot bug #1072

Merged
merged 11 commits into from
Nov 27, 2024
Merged

updating error message and fixing retry snapshot bug #1072

merged 11 commits into from
Nov 27, 2024

Conversation

prklm10
Copy link
Contributor

@prklm10 prklm10 commented Nov 22, 2024

updating error message and fixing retry snapshot bug

@prklm10 prklm10 requested a review from a team as a code owner November 22, 2024 11:29
@prklm10 prklm10 added 🐛 bug Something isn't working 🧹 maintenance General maintenance and removed 🐛 bug Something isn't working labels Nov 22, 2024
src/utils.js Outdated
throw error;
}
log.warn(`Retrying Story: ${args.snapshotName}`);
log.warn(`Retrying Story: ${args?.snapshotName}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pass the message from out, this will print incorrect line when withPage errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

src/utils.js Outdated
// Add snapshotName to the error message
const snapshotName = args?.snapshotName;
if (snapshotName) {
error.message = `${error.message} - Snapshot Name: ${snapshotName}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldnt this be Snapshot name followed by error.message ? wouldnt that make it easy to understand snapshot in case of long error messages ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the original comment is not handled. THe message should be passed from outside. Considering "Retrying Story" makes no sense if we are opening preview page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passed form inside try when attempt number > 0.

test fix

test fix

test fix

test fix

test fix

test fix
@@ -173,12 +173,15 @@ describe('percy storybook', () => {

await expectAsync(storybook(['http://localhost:8000']))
// message contains the client stack trace
.toBeRejectedWithError(/^Story Error\n.*\/iframe\.html.*$/s);
.toBeRejectedWithError(/^Snapshot Name:/s);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. Snapshot name is getting appended to the error.

@prklm10 prklm10 merged commit 051819a into master Nov 27, 2024
6 checks passed
@prklm10 prklm10 deleted the PER-4029 branch November 27, 2024 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 maintenance General maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants