Skip to content

Update the file used with the xfa_bug1720182 test-case #14547

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

Merged
merged 2 commits into from
Feb 9, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

The file used in this test-case is identical to, i.e. the md5 entry perfectly matches, the file used with the xfa_bug1716380 test-case.
While it's obviously fine to use the same PDF document in different reference-tests, note how we e.g. have both eq and text tests for one document, we should always avoid adding duplicate files in the test/pdfs/ folder.

@Snuffleupagus
Copy link
Collaborator Author

/botio xfatest

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/03af9f8c84d9421/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/2e6ac3dd2c1bc56/output.txt

The file used in this test-case is *identical* to, i.e. the md5 entry perfectly matches, the file used with the `xfa_bug1716380` test-case.
While it's obviously fine to use the same PDF document in different reference-tests, note how we e.g. have both `eq` and `text` tests for one document, we should always avoid adding *duplicate* files in the `test/pdfs/` folder.
@Snuffleupagus Snuffleupagus changed the title Update the file used with the xfa_bug172018 test-case Update the file used with the xfa_bug1720182 test-case Feb 8, 2022
@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/03af9f8c84d9421/output.txt

Total script time: 10.12 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 1

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

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/2e6ac3dd2c1bc56/output.txt

Total script time: 12.22 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 1

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

@marco-c
Copy link
Contributor

marco-c commented Feb 8, 2022

Maybe we should have some kind of linting to avoid this kind of problems.

@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Feb 8, 2022

Maybe we should have some kind of linting to avoid this kind of problems.

Well, I didn't exactly find this manually :-)
Locally I've got a patch for the Driver to fail on duplicated files, however all the existing cases needed to be fixed first such that we don't introduce "new" failures in the reference tests. Recently I've been going through and fixing the "broken" cases, but not until now has it actually been feasible to enforce this.

While it's obviously fine to use the same PDF document in different reference-tests, note how we e.g. have both `eq` and `text` tests for one document, we should always avoid adding *duplicate* files in the `test/pdfs/` folder.
@Snuffleupagus
Copy link
Collaborator Author

/botio browsertest

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

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/4e7aae2e3e65ed9/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/f4732137930646e/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/f4732137930646e/output.txt

Total script time: 19.41 mins

  • Regression tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented Feb 8, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/4e7aae2e3e65ed9/output.txt

Total script time: 20.02 mins

  • Regression tests: FAILED
  different ref/snapshot: 8

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

@timvandermeij timvandermeij merged commit d57f3a1 into mozilla:master Feb 9, 2022
@timvandermeij
Copy link
Contributor

Nice find!

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.

4 participants