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.

Sign up for GitHub

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

Jump to bottom

Fix E2E test report upload URL #2776

Merged
merged 2 commits into from
Jun 28, 2023
Merged

Fix E2E test report upload URL #2776

merged 2 commits into from
Jun 28, 2023

Conversation

mvitale1989
Copy link
Member

Summary

This PR fixes a bug in the E2E test report upload script, which caused the report to be inaccessible for Windows E2E tests.

Ticket Link

https://mattermost.atlassian.net/browse/CLD-5909

Release Note

NONE

devinbinnie
devinbinnie approved these changes Jun 28, 2023
View reviewed changes
Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Approving to unblock when this is good to merge.

@mvitale1989 mvitale1989 marked this pull request as ready for review June 28, 2023 14:40
@yasserfaraazkhan
Copy link
Contributor

/e2e-test

@mattermost-build
Copy link
Contributor

Successfully triggered e2e desktop testing!
https://git.internal.mattermost.com/qa/desktop-e2e-testing/-/pipelines/348979

@mvitale1989
Copy link
Member Author

mvitale1989 commented Jun 28, 2023 •
edited
Loading

I also manually launched a windows pipeline windows pipeline, to verify that the produced report links are valid

@yasserfaraazkhan
Copy link
Contributor

@mvitale1989

https://community.mattermost.com/core/pl/kyh1us7syjr19m9tnsu7oqoxzw

maybe the report is not building. It's accessible so this fix is working.

https://mattermost-cypress-report.s3.amazonaws.com/348970-desktop-pr-windows/amd64-cld-5909-fix-e2etest-report-abd9e6c/mochawesome.html

@mvitale1989
Copy link
Member Author

That report is only partially working, but it should be fixed by this commit that i already pushed: c1c2dfe

tldr: the replace() function only substituted one backslash; so e.g. amd64-cld-5909-fix-e2etest-report-abd9e6c\mochawesome.html gets properly fixed and you can see it loads, but e.g. amd64-cld-5909-fix-e2etest-report-abd9e6c\assets\app.css has more than one backslash, so it's still broken after using replace(). Hopefully replaceAll() fixes this, waiting for the Windows pipeline to finish, to verify this

@yasserfaraazkhan
Copy link
Contributor

thank you Mario

@mvitale1989
Copy link
Member Author

It worked 🎉 https://community.mattermost.com/core/pl/7hzbdpcgnjr4bkfsku5dohryay

@mvitale1989 mvitale1989 merged commit 1da06bc into master Jun 28, 2023
8 checks passed
@mvitale1989 mvitale1989 deleted the cld-5909-fix-e2etest-report branch June 28, 2023 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yasserfaraazkhan yasserfaraazkhan yasserfaraazkhan approved these changes

@devinbinnie devinbinnie devinbinnie approved these changes

Assignees
No one assigned
Labels
release-note-none
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mvitale1989 @yasserfaraazkhan @mattermost-build @devinbinnie @mm-cloud-bot