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

Remove date from testData file #2079

Closed
wants to merge 1 commit into from

Conversation

piponazo
Copy link
Collaborator

@piponazo piponazo commented Feb 8, 2022

In #2053 a new test was added and since then I was obtaining failures when running the tests locally.

When running the tests locally, the DateTimeOriginal key is printed like this:

Exif.Photo.DateTimeOriginal 2022:01:04 10:41:01  2022:01:04 10:41:01

While the expectation was:

Exif.Photo.DateTimeOriginal 2022:01:04 09:41:01 2022:01:04 09:41:01

It seems that such time is printed in local time. Since that field is not relevant for the test added in #2053, I think we can just remove it.

@codecov
Copy link

codecov bot commented Feb 8, 2022

Codecov Report

Merging #2079 (b91924e) into main (4ae42b1) will decrease coverage by 0.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2079      +/-   ##
==========================================
- Coverage   62.08%   61.87%   -0.22%     
==========================================
  Files          96       96              
  Lines       19153    19059      -94     
  Branches     9798     9782      -16     
==========================================
- Hits        11891    11792      -99     
- Misses       4967     4984      +17     
+ Partials     2295     2283      -12     
Impacted Files Coverage Δ
src/safe_op.hpp 69.23% <0.00%> (-27.65%) ⬇️
include/exiv2/slice.hpp 69.64% <0.00%> (-21.89%) ⬇️
include/exiv2/error.hpp 60.71% <0.00%> (-4.81%) ⬇️
src/utils.cpp 38.46% <0.00%> (-1.93%) ⬇️
include/exiv2/value.hpp 82.68% <0.00%> (-0.56%) ⬇️
src/enforce.hpp 75.00% <0.00%> (+2.77%) ⬆️
src/getopt.cpp 67.30% <0.00%> (+5.76%) ⬆️
include/exiv2/metadatum.hpp 88.88% <0.00%> (+16.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ae42b1...b91924e. Read the comment docs.

@postscript-dev
Copy link
Collaborator

postscript-dev commented Feb 8, 2022 via email

@piponazo
Copy link
Collaborator Author

piponazo commented Feb 8, 2022

@piponazohttps://github.com/piponazo: I am happy to review this but it will have to wait until the weekend.

Sure, whenever you have some spare time 😉

@postscript-dev
Copy link
Collaborator

@piponazo: When I added #2053, I included every tag that was in the XMP IPTC Metadata Standard 2021.1 - which doesn't include Exif.Photo.DateTimeOriginal.

The problems in this issue (and in #2083) is that the tags are passing through conversion functions which add or update some of the tags. The error is caused by one of the conversion functions using the local time.

This conversion problem has come up before though in #367 (the comments start here). In that issue, the solution was to add:

env = {
    'TZ': 'UTC'
}

to the Python test (see tests/bash_tests/test_issue_1054.py).

As it looks as though the same problem could reoccur with other files in the future, perhaps it is better to set the default test suite environment to UTC?

@piponazo
Copy link
Collaborator Author

@postscript-dev yes, I think I will close this PR because we definitely need to take a deeper look to the root issue.

@piponazo piponazo closed this Feb 13, 2022
@piponazo piponazo deleted the main_ammendTestDependingOnLocalTime branch February 16, 2022 07:57
@piponazo piponazo mentioned this pull request Feb 16, 2022
@postscript-dev
Copy link
Collaborator

postscript-dev commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants