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

Add auto-detect of attachments extension [playwright] #470

Closed
wants to merge 5 commits into from

Conversation

elipskiy
Copy link

The Playwright Reporter missed attachments whose extensions could not detect from content type.

Add auto detect file extensions from the file name.

Checklist

@github-actions github-actions bot added theme:api Javascript API related issue theme:playwright labels Jul 27, 2022
@elipskiy elipskiy requested a review from baev July 29, 2022 11:17
@baev baev requested a review from vovsemenv August 9, 2022 12:22
@vovsemenv
Copy link
Collaborator

@elipskiy Thank you for contribution! 🙏

@vovsemenv vovsemenv added the type:improvement Improvement or request label Aug 9, 2022
@vovsemenv
Copy link
Collaborator

vovsemenv commented Aug 9, 2022

@vovsemenv
Copy link
Collaborator

@baev we ready to go

@elipskiy
Copy link
Author

@baev please, merge this pr

@elipskiy
Copy link
Author

@baev @viclovsky

@baev
Copy link
Member

baev commented Oct 18, 2022

The PR isn't merged yet due to the following concerns:

  1. Attachment file extension isn't really necessary as it will be detected automatically during report generation if correct content type is supplied.
  2. Besides that, we can use file content to detect correct content type (by using magic bytes) -- it's already implemented in both Allure Report & Allure TestOps
  3. It makes sense to reuse existing file extension though. But I'm not sure about extname -- there are plenty of complex file extensions like tar.gz , and it may be confusing for some users (or not)
  4. Why playwright? The main change is located in commons, so you need to add test to commons module.

@baev baev removed their request for review February 26, 2024 11:50
@baev
Copy link
Member

baev commented May 21, 2024

fixed via #956

@baev baev closed this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme:api Javascript API related issue theme:playwright type:improvement Improvement or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants