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 the pdkids PDF document to the repository #18902

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

Snuffleupagus
Copy link
Collaborator

Given that the sub-title of that document is "Public domain texts for young people." and that the images have clear sources at the end of the document, it should (hopefully) be OK to add it to the repository rather than relying on a linked test-case.

Given that the sub-title of that document is "Public domain texts for young people." and that the images have clear sources at the end of the document, it should (hopefully) be OK to add it to the repository rather than relying on a linked test-case.
@Snuffleupagus
Copy link
Collaborator Author

/botio browsertest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_browsertest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/3ca65c5c6101101/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_browsertest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/6b751fea1d0c02e/output.txt

@marco-c
Copy link
Contributor

marco-c commented Oct 15, 2024

@calixteman and I were talking about this, maybe we could store the PDFs in a separate repo. This would avoid slowing things down in the main pdf.js repo by having fewer binary files.

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/3ca65c5c6101101/output.txt

Total script time: 20.86 mins

  • Regression tests: FAILED
  different ref/snapshot: 18
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/3ca65c5c6101101/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/6b751fea1d0c02e/output.txt

Total script time: 30.45 mins

  • Regression tests: FAILED
  different ref/snapshot: 2

Image differences available at: http://54.193.163.58:8877/6b751fea1d0c02e/reftest-analyzer.html#web=eq.log

@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Oct 15, 2024

@calixteman and I were talking about this, maybe we could store the PDFs in a separate repo. This would avoid slowing things down in the main pdf.js repo by having fewer binary files.

A few observations/questions:

  • The reason that we have linked test-cases is because historically we didn't want to deal with any potential legal issues of re-distributing third-party PDFs.
  • If we have a separate repository, could that make it more difficult for users to get the test-suite setup and running locally?
  • Ideally we'd only have reduced test-cases for the PDFs, but unfortunately it's often difficult to manually re-create all the brokenness found in real-world PDFs.

In any case, replacing a linked test-case as this PR does seems generally desirable regardless of any other (future) changes.

@marco-c
Copy link
Contributor

marco-c commented Oct 15, 2024

Yes, it's something to consider for later, this PR looks good as it is.

@Snuffleupagus
Copy link
Collaborator Author

Yes, it's something to consider for later, this PR looks good as it is.

Given that this PR contains a small test-only change, I'll consider that as approval to just land this :-)

@Snuffleupagus Snuffleupagus merged commit 689ffda into mozilla:master Oct 15, 2024
6 checks passed
@Snuffleupagus Snuffleupagus deleted the pdkids-rm-linked-test branch October 15, 2024 20:15
@Snuffleupagus Snuffleupagus removed the request for review from calixteman October 15, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants