Conversation
"copy_old_to_new" was checking that the field existed previously, but not that it was non-empty. This doesn't cover every possible scenario, and there are likely other cases of invalid metadata in RGB TIFFs that we may need to handle in the future, but we can deal with it when it comes up.
aelkiss
commented
Sep 18, 2025
| ok($validate->succeeded()); | ||
| }; | ||
|
|
||
| it "remediates empty DateTime in a bitonal TIFF file" => sub { |
Member
Author
There was a problem hiding this comment.
This test (with its associated fixture) passed without any changes to the ImageRemediate code, but I think it's worth keeping as a regression test.
aelkiss
commented
Sep 18, 2025
| defined $self->{oldFields}->{$oldFieldName} and | ||
| not defined $self->{newFields}->{$newFieldName} | ||
| defined $self->{oldFields}->{$oldFieldName} | ||
| and $self->{oldFields}->{$oldFieldName} ne '' |
Member
Author
There was a problem hiding this comment.
This method (copy_old_to_new) is called here:
feed/lib/HTFeed/Stage/ImageRemediate.pm
Line 1165 in 6b1f116
copy_old_to_new but I don't know if we've run into that exactly.
liseli
approved these changes
Sep 18, 2025
liseli
left a comment
liseli
left a comment
There was a problem hiding this comment.
These changes look good. I appreciate seeing the different tests that consider various scenarios for image validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This ended up being a bit tricky to track down because the error didn't give a useful stack trace, and the culprit is buried in the rather long
convert_tiff_to_jpeg2000method.This was indeed a one-line fix (6b1f116#diff-0d491008f8069f2e647a11563976e76d9a8fd71e522c3efbb3b9384b58b93ae6R175), but getting there was a bit tricky in terms of figuring out exactly what triggered the issue, creating a test fixture, and then actually finding the source of the problem.
It may be worth thinking a bit about what we do with ImageRemediate in the future – in particular if we want to make major changes to this in the future we should do some refactoring and unit testing there first.
6b1f116 is the commit with the actual changes. Some comments inline.