Skip to content

G-API: Removing G-API test code that is a reflection of ts module#20995

Merged
alalek merged 3 commits intoopencv:4.xfrom
mpashchenkov:mp/ocv-gapi-tdp-skip
Nov 16, 2021
Merged

G-API: Removing G-API test code that is a reflection of ts module#20995
alalek merged 3 commits intoopencv:4.xfrom
mpashchenkov:mp/ocv-gapi-tdp-skip

Conversation

@mpashchenkov
Copy link
Copy Markdown
Contributor

@mpashchenkov mpashchenkov commented Nov 3, 2021 •
edited
Loading

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

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

Build Configuration

force_builders=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.4.1:20.04
build_image:Custom Win=openvino-2021.4.1
build_image:Custom Mac=openvino-2021.4.1

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

alalek
alalek reviewed Nov 3, 2021
View reviewed changes
modules/gapi/test/cpu/gapi_ocv_stateful_kernel_tests.cpp Outdated
auto ccomp = c.compileStreaming(cv::compile_args(pkg));

auto path = findDataFile("cv/video/768x576.avi");
auto path = findDataFile("cv/video/768x576.avi", false);
Copy link
Copy Markdown
Member

@alalek alalek Nov 3, 2021 •
edited
Loading

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

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Nov 8, 2021

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.

Copy link
Copy Markdown
Member

@alalek alalek Nov 8, 2021

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

Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work Nov 8, 2021

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:

  1. throw SkipExpection if no OPENCV_TEST_DATA_PATH is set
  2. if it set then fileData MUST 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?

Copy link
Copy Markdown
Member

@alalek alalek Nov 8, 2021 •
edited
Loading

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).

Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work Nov 8, 2021

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

@alexgiving alexgiving self-requested a review November 4, 2021 19:05
@sivanov-work sivanov-work mentioned this pull request Nov 8, 2021
Merged
6 tasks
@mpashchenkov mpashchenkov changed the title G-API: Adding false for FindDataFile G-API: Removing G-API test code that is a reflection of ts module Nov 15, 2021
@mpashchenkov
Copy link
Copy Markdown
Contributor Author

mpashchenkov commented Nov 15, 2021

@alalek, removed initTestDataPath() logic from gapi module. ts module set OPENCV_TEST_DATA_PATH variable for all tests.

sivanov-work
sivanov-work approved these changes Nov 16, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work left a 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

@mpashchenkov
Copy link
Copy Markdown
Contributor Author

mpashchenkov commented Nov 16, 2021

@alalek, can it be merged? Is it the expected test behavior for you?

@sivanov-work sivanov-work mentioned this pull request Nov 16, 2021
Merged
6 tasks
alalek
alalek approved these changes Nov 16, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@alalek alalek merged commit 2f6d2b0 into opencv:4.x Nov 16, 2021
@alalek alalek mentioned this pull request Dec 30, 2021
Merged
@alalek alalek mentioned this pull request Feb 22, 2022
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
@alalek
Merge pull request opencv#20995 from mpashchenkov:mp/ocv-gapi-tdp-skip
5849e24 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek approved these changes

@sivanov-work sivanov-work sivanov-work approved these changes

@alexgiving alexgiving Awaiting requested review from alexgiving

Assignees

@sivanov-work sivanov-work

Labels

category: g-api / gapi test

Projects

None yet

Milestone

4.5.5

Development

Successfully merging this pull request may close these issues.

4 participants

@mpashchenkov @alalek @alexgiving @sivanov-work