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

Fix test_pr_1905.py for native 2019msvc64 (#2108) #2112

Merged
merged 2 commits into from
Feb 20, 2022

Conversation

postscript-dev
Copy link
Collaborator

Closes #2108.

@postscript-dev postscript-dev added testing Anything related to the tests and their infrastructure windows Windows Specific issues labels Feb 20, 2022
@postscript-dev postscript-dev added this to the v1.00 milestone Feb 20, 2022
@postscript-dev postscript-dev self-assigned this Feb 20, 2022
piponazo
piponazo previously approved these changes Feb 20, 2022
Copy link
Collaborator

@piponazo piponazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on my Windows PC and it works 👏

Copy link
Collaborator

@clanmills clanmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful fix, @postscript-dev.

@codecov
Copy link

codecov bot commented Feb 20, 2022

Codecov Report

Merging #2112 (4a6d44a) into main (5d86044) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2112   +/-   ##
=======================================
  Coverage   63.80%   63.80%           
=======================================
  Files          96       96           
  Lines       19171    19171           
  Branches     9772     9772           
=======================================
  Hits        12232    12232           
  Misses       4658     4658           
  Partials     2281     2281           

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 5d86044...4a6d44a. Read the comment docs.

@postscript-dev
Copy link
Collaborator Author

@piponazo:
Thanks for the quick response!

Last year, I noticed a problem related to this issue that I think is worth mentioning.

The Exiv2 mechanism to create tag webpages from the source code is only run when we publish the website. This means that when we make changes, the CI will not catch any errors for us. To create the webpages is simple.

  1. Set the EXIV2HOME environment variable (e.g., /home/postscript-dev/exiv2_postscript-dev)
  2. In the doc/templates directory, run make

The build instructions are missing from README.md but I plan to add them.

If you have the time, could you look at the CI?

Copy link
Member

@hassec hassec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep test/data for input data and use test/data/test_reference_files for reference output?

@piponazo
Copy link
Collaborator

Can we keep test/data for input data and use test/data/test_reference_files for reference output?

I think it would be a good idea to write this down in the wiki or in the test/README document to give instructions to people about some of the practices we have been discussing lately about writting tests.

The Exiv2 mechanism to create tag webpages from the source code is only run when we publish the website. This means that when we make changes, the CI will not catch any errors for us. To create the webpages is simple.

@postscript-dev whenever you create the PR with the instructions, please do it in the main repository and I'll happily try to adapt our CI jobs to make this run on every PR.

@postscript-dev postscript-dev merged commit d17b7e4 into Exiv2:main Feb 20, 2022
@postscript-dev postscript-dev deleted the fix-test_pr_1905_win_build branch March 3, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Anything related to the tests and their infrastructure windows Windows Specific issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_pr_1905.TestExifTagsInTaglist depends on cut command
4 participants