| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
G-API: Removing G-API test code that is a reflection of ts module #20995
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
Conversation
| @@ -192,7 +192,7 @@ TEST(StatefulKernel, StateIsAutoResetForNewStream) | |||
| // Compilation & testing | |||
| auto ccomp = c.compileStreaming(cv::compile_args(pkg)); | |||
|
|
|||
| auto path = findDataFile("cv/video/768x576.avi"); | |||
| auto path = findDataFile("cv/video/768x576.avi", false); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
false should not be used for files from opencv_extra.
false is used for files from external additional optional share ONLY.
ref: #14847
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a check for OPENCV_TEST_DATA_PATH for emptiness? FAILED tests without OPENCV_TEST_DATA_PATH can shock people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FAILED tests without OPENCV_TEST_DATA_PATH can shock people
This is expected behavior.
Launching tests without required test data doesn't make sense. They should not pass at least.
https://github.com/opencv/opencv/wiki/QA_in_OpenCV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alalek can you clarify this assertion? i faced with the same behavior in my PR and got several comments.
Could you take a look please?
https://github.com/opencv/opencv/pull/20901/files#diff-df9d13b0d9035428deb2704ecdd722cb4a3193ba653554215768156fffec1f62R43
TO my mind it is expected behavior for:
- throw SkipExpection if no
OPENCV_TEST_DATA_PATHis set - if it set then
fileDataMUST follows up to correct file. If path is incorrect that it MUST fail test.
The question is: do we need to check initTestDataPathSilent before fileData failed - or in another words: is my PR (by link above) behavior correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw SkipExpection if no OPENCV_TEST_DATA_PATH is set
OPENCV_TEST_DATA_PATH points to required test data (opencv_extra/testdata).
Test which requires data from opencv_extra must fail in that case.
Passed test (even through skip) is wrong behavior.
Test is skipped if optional test data is missing, e.g. DNN models (it is done because full dataset is large).
G-API tests SHOULD NOT use OPENCV_TEST_DATA_PATH directly.
G-API tests SHOULD use OPENCV_DNN_TEST_DATA_PATH once during initialization (see "dnn" module).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, got it. thanks for explanations
mpashchenkov
force-pushed
the
mp/ocv-gapi-tdp-skip
branch
from
11cbee7 to
6bc4a5e
Compare
mpashchenkov
force-pushed
the
mp/ocv-gapi-tdp-skip
branch
from
6bc4a5e to
dc578d9
Compare
mpashchenkov
force-pushed
the
mp/ocv-gapi-tdp-skip
branch
from
d0f454d to
9fccec8
Compare
mpashchenkov
changed the title
|
@alalek, removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, if UTs behave as expected
It looks like other subsequent pending PRs (oneVPL) with new unit tests required to be rebased after this PR merged
|
@alalek, can it be merged? Is it the expected test behavior for you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
G-API: Removing G-API test code that is a reflection of ts module * gapi: don't hijack testing infrastructure * Removed initDataPath functionality (ts module exists) * Removed false for ocv_extra data from findDataFile Co-authored-by: Alexander Alekhin <alexander.a.alekhin@gmail.com>
This PR sets the second parameter of FindDataFile to false for tests of G-API. This handles the problem with failed tests which aren't skipped.
Also removing
initTestDataPath()logic.Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.
Build Configuration