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

tests: fix parsing w/ path containing "-pa" or "-pS" #2826

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

pinotree
Copy link
Contributor

The OutputTagExtract test case for tiff_test runs exiv2 two times, with -pa and -pS, and parses their outputs. To know which output to parse, it checks for -pa and -pS in the string of the command being run; considering that the command string contains the full path to the test data, which is a subdirectory of the sources, this means that a wrong parser will be used in case the full source path contains any of -pa or -pS (e.g. /build/some-path/exiv2/...).

Cheap fix for this: since -pa/-pS are options in the command string, check for them using spaces around.

The OutputTagExtract test case for tiff_test runs exiv2 two times,
with "-pa" and "-pS", and parses their outputs. To know which output to
parse, it checks for "-pa" and "-pS" in the string of the command being
run; considering that the command string contains the full path to the
test data, which is a subdirectory of the sources, this means that a
wrong parser will be used in case the full source path contains any of
"-pa" or "-pS" (e.g. "/build/some-path/exiv2/...").

Cheap fix for this: since "-pa"/"-pS" are options in the command string,
check for them using spaces around.
@ghost
Copy link

ghost commented Nov 12, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Merging #2826 (db47dfb) into main (9002797) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2826   +/-   ##
=======================================
  Coverage   64.00%   64.00%           
=======================================
  Files         103      103           
  Lines       22324    22324           
  Branches    10827    10827           
=======================================
  Hits        14288    14288           
  Misses       5818     5818           
  Partials     2218     2218           

@kmilos kmilos merged commit 5f4ffdd into Exiv2:main Nov 12, 2023
59 checks passed
@kmilos
Copy link
Collaborator

kmilos commented Nov 12, 2023

@Mergifyio backport 0.28.x

Copy link
Contributor

mergify bot commented Nov 12, 2023

backport 0.28.x

✅ Backports have been created

@kmilos
Copy link
Collaborator

kmilos commented Nov 12, 2023

Thanks!

@pinotree pinotree deleted the tests-fix-path-check branch November 12, 2023 18:14
@kmilos kmilos added the testing Anything related to the tests and their infrastructure label Jan 3, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants