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: Use relative path for attachment reference in HTML report #275

Merged

Conversation

ntduyet
Copy link
Contributor

@ntduyet ntduyet commented Aug 18, 2022

This PR is to fix the broken images issue #274.

Changes

  • Use relativeUrl to construct the return value in ResultFile.exportPayload(id:, fileName:)
  • Add unit tests to avoid being broken in the future

@ntduyet ntduyet force-pushed the duyet/fix-images-url-in-html-report branch from 0a8b360 to 043e6e1 Compare August 19, 2022 10:17
Copy link
Member

@tylervick tylervick left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, and especially the unit test coverage 🎉

@ntduyet ntduyet changed the title Fix: Use relative path when referencing images in HTML report Fix: Use relative path for attachment reference in HTML report Aug 20, 2022
@ntduyet
Copy link
Contributor Author

ntduyet commented Aug 20, 2022

Hi @tylervick, thanks for review this PR. Would you mind triggering the workflow check again? I've added more tests to cover text attachment as well 🙏

@tylervick
Copy link
Member

Apologies for the delay - running this now. Happy to merge when you're ready - I'll also plan to do a patch version bump

@ntduyet
Copy link
Contributor Author

ntduyet commented Aug 22, 2022

I think I'm done with the updates. Please proceed to merge it when you have time. Thank you.

@tylervick tylervick merged commit 6a1f162 into XCTestHTMLReport:main Aug 22, 2022
@ntduyet ntduyet deleted the duyet/fix-images-url-in-html-report branch August 22, 2022 22:41
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.

None yet

2 participants