Add tests for bug 474670216 [pdfium : main]
Aryan Krishnan (Gerrit)
Aryan Krishnan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Aryan Krishnan
| Commit-Queue | +1 |
1 comment
ASSERT_DEATH(OpenDocument(filename), "");Would probably want a better solution than this. This test alone takes about 40 seconds on my machine.
Related details
- Andy Phan
- Lei Zhang
- Tom Sepez
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Tom Sepez added 3 comments![]( "Open in Gerrit")
TEST_F(FPDFPPOEmbedderTest, Bug474670216) {calling this DISABLED_Bug474670216 allows the test to be skipped. Removethe DISABLED_ prefix when landing the CL that fixes the bug.
static const char* kCrashingPDFs[] = {these are now all working pdfs (at least in a universe where this is fixed).
for (const char* filename : kCrashingPDFs) {
ASSERT_DEATH(OpenDocument(filename), "");
}and just strike this.
Related details
- Andy Phan
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 1 comment![]( "Open in Gerrit")
TEST_F(FPDFPPOEmbedderTest, Bug474670216) {calling this DISABLED_Bug474670216 allows the test to be skipped. Removethe DISABLED_ prefix when landing the CL that fixes the bug.
@tse...@chromium.org Issue is, if this is is disabled, how can you verify if just one of them is solved, without re enabling it?
I believe that the issues were different across all of these PDFs.
Maybe have all tests as separate so that we can enable and disable as we need?
Related details
- Andy Phan
- Lei Zhang
- Tom Sepez
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Tom Sepez added 1 comment![]( "Open in Gerrit")
TEST_F(FPDFPPOEmbedderTest, Bug474670216) {Aryan Krishnancalling this DISABLED_Bug474670216 allows the test to be skipped. Removethe DISABLED_ prefix when landing the CL that fixes the bug.
@tse...@chromium.org Issue is, if this is is disabled, how can you verify if just one of them is solved, without re enabling it?
I believe that the issues were different across all of these PDFs.
Maybe have all tests as separate so that we can enable and disable as we need?
- Andy Phan
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 3 comments![]( "Open in Gerrit")
Aryan Krishnancalling this DISABLED_Bug474670216 allows the test to be skipped. Removethe DISABLED_ prefix when landing the CL that fixes the bug.
Tom Sepez@tse...@chromium.org Issue is, if this is is disabled, how can you verify if just one of them is solved, without re enabling it?
I believe that the issues were different across all of these PDFs.
Maybe have all tests as separate so that we can enable and disable as we need?
yep. Separate.
Done
these are now all working pdfs (at least in a universe where this is fixed).
Done
for (const char* filename : kCrashingPDFs) {
ASSERT_DEATH(OpenDocument(filename), "");
}- Andy Phan
- Lei Zhang
- Tom Sepez
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Tom Sepez voted Code-Review+1![]( "Open in Gerrit")
- Andy Phan
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 1 comment![]( "Open in Gerrit")
Running this so it doesn't have to re-run on submit.
Related details
- Andy Phan
- Lei Zhang
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Phan (Gerrit)
Andy Phan added 1 comment![]( "Open in Gerrit")
This CL is adding 7 MB worth of test PDFs. Most PDFs in testing/resources/ are under 100 KB. It would be nice to keep it this way. Some options:
1. Look into the PDFs and try to minimize its contents. Most time-consuming.
2. Remove some of these PDFs. They might have the same root cause, especially if they have the same stack trace. Adding more of the PDFs for the same root cause would be redundant. They can be added back as needed if there's a different cause.
3. A mix of (1) and (2).
Investigating some of the PDFs might help decide which is better.
Related details
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 1 comment![]( "Open in Gerrit")
This CL is adding 7 MB worth of test PDFs. Most PDFs in testing/resources/ are under 100 KB. It would be nice to keep it this way. Some options:
1. Look into the PDFs and try to minimize its contents. Most time-consuming.
2. Remove some of these PDFs. They might have the same root cause, especially if they have the same stack trace. Adding more of the PDFs for the same root cause would be redundant. They can be added back as needed if there's a different cause.
3. A mix of (1) and (2).Investigating some of the PDFs might help decide which is better.
Will look at optimizing these first: They seem to differ quite a bit in terms of root cause and the stack trace.
Or perhaps, instead of testing all cases of this bug, we can test only a few.
What do you think?
I tried using a compressor and that fails to compress due to some malformed objects.
Related details
- Andy Phan
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Phan (Gerrit)
Andy Phan added 1 comment![]( "Open in Gerrit")
Aryan KrishnanThis CL is adding 7 MB worth of test PDFs. Most PDFs in testing/resources/ are under 100 KB. It would be nice to keep it this way. Some options:
1. Look into the PDFs and try to minimize its contents. Most time-consuming.
2. Remove some of these PDFs. They might have the same root cause, especially if they have the same stack trace. Adding more of the PDFs for the same root cause would be redundant. They can be added back as needed if there's a different cause.
3. A mix of (1) and (2).Investigating some of the PDFs might help decide which is better.
Will look at optimizing these first: They seem to differ quite a bit in terms of root cause and the stack trace.
Or perhaps, instead of testing all cases of this bug, we can test only a few.
What do you think?
I tried using a compressor and that fails to compress due to some malformed objects.
The bug report claims they're all unique stacktraces. Hmm, outside of just trying to minimize the contents, not sure then. Any thoughts, thestig@?
There's corpus tests, but I think those always compare pixel results.
Related details
- Aryan Krishnan
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Lei Zhang added 1 comment![]( "Open in Gerrit")
Aryan KrishnanThis CL is adding 7 MB worth of test PDFs. Most PDFs in testing/resources/ are under 100 KB. It would be nice to keep it this way. Some options:
1. Look into the PDFs and try to minimize its contents. Most time-consuming.
2. Remove some of these PDFs. They might have the same root cause, especially if they have the same stack trace. Adding more of the PDFs for the same root cause would be redundant. They can be added back as needed if there's a different cause.
3. A mix of (1) and (2).Investigating some of the PDFs might help decide which is better.
Andy PhanWill look at optimizing these first: They seem to differ quite a bit in terms of root cause and the stack trace.
Or perhaps, instead of testing all cases of this bug, we can test only a few.
What do you think?
I tried using a compressor and that fails to compress due to some malformed objects.
The bug report claims they're all unique stacktraces. Hmm, outside of just trying to minimize the contents, not sure then. Any thoughts, thestig@?
There's corpus tests, but I think those always compare pixel results.
Yes, I would prefer not to check 7 MB of binary blobs. The PDFium repo, without .git and all the things pulled in via DEPS, is only 51 MB.
These PDFs are probably just garbage generated by a fuzzer. It's the most interesting garbage, but it's not worth checking in or optimizing. I would just skip this CL, fix the bugs and call it done. Especially considering the failure is mostly just nullptr derefs and such.
Related details
- Aryan Krishnan
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 1 comment![]( "Open in Gerrit")
Aryan KrishnanThis CL is adding 7 MB worth of test PDFs. Most PDFs in testing/resources/ are under 100 KB. It would be nice to keep it this way. Some options:
1. Look into the PDFs and try to minimize its contents. Most time-consuming.
2. Remove some of these PDFs. They might have the same root cause, especially if they have the same stack trace. Adding more of the PDFs for the same root cause would be redundant. They can be added back as needed if there's a different cause.
3. A mix of (1) and (2).Investigating some of the PDFs might help decide which is better.
Andy PhanWill look at optimizing these first: They seem to differ quite a bit in terms of root cause and the stack trace.
Or perhaps, instead of testing all cases of this bug, we can test only a few.
What do you think?
I tried using a compressor and that fails to compress due to some malformed objects.
Lei ZhangThe bug report claims they're all unique stacktraces. Hmm, outside of just trying to minimize the contents, not sure then. Any thoughts, thestig@?
There's corpus tests, but I think those always compare pixel results.
Yes, I would prefer not to check 7 MB of binary blobs. The PDFium repo, without .git and all the things pulled in via DEPS, is only 51 MB.
These PDFs are probably just garbage generated by a fuzzer. It's the most interesting garbage, but it's not worth checking in or optimizing. I would just skip this CL, fix the bugs and call it done. Especially considering the failure is mostly just nullptr derefs and such.
Oh, so are you suggesting not to check in tests, just use pdfium_test or something of the sort to test it locally before uploading? Ok will update.
Related details
- Andy Phan
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aryan Krishnan (Gerrit)
Aryan Krishnan added 1 comment![]( "Open in Gerrit")
Aryan KrishnanThis CL is adding 7 MB worth of test PDFs. Most PDFs in testing/resources/ are under 100 KB. It would be nice to keep it this way. Some options:
1. Look into the PDFs and try to minimize its contents. Most time-consuming.
2. Remove some of these PDFs. They might have the same root cause, especially if they have the same stack trace. Adding more of the PDFs for the same root cause would be redundant. They can be added back as needed if there's a different cause.
3. A mix of (1) and (2).Investigating some of the PDFs might help decide which is better.
Andy PhanWill look at optimizing these first: They seem to differ quite a bit in terms of root cause and the stack trace.
Or perhaps, instead of testing all cases of this bug, we can test only a few.
What do you think?
I tried using a compressor and that fails to compress due to some malformed objects.
Lei ZhangThe bug report claims they're all unique stacktraces. Hmm, outside of just trying to minimize the contents, not sure then. Any thoughts, thestig@?
There's corpus tests, but I think those always compare pixel results.
Aryan KrishnanYes, I would prefer not to check 7 MB of binary blobs. The PDFium repo, without .git and all the things pulled in via DEPS, is only 51 MB.
These PDFs are probably just garbage generated by a fuzzer. It's the most interesting garbage, but it's not worth checking in or optimizing. I would just skip this CL, fix the bugs and call it done. Especially considering the failure is mostly just nullptr derefs and such.
Oh, so are you suggesting not to check in tests, just use pdfium_test or something of the sort to test it locally before uploading? Ok will update.
Abandoning this CL, have moved the parent CL to be rebased on main.
Aryan Krishnan (Gerrit)
Aryan Krishnan abandoned this change.
Aryan Krishnan abandoned this change![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |