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

Bug fixes #95

Merged
merged 3 commits into from
Mar 29, 2019
Merged

Bug fixes #95

merged 3 commits into from
Mar 29, 2019

Conversation

vishakha041
Copy link
Contributor

Fixes to some reported issues like error on empty results

@vishakha041 vishakha041 requested a review from luisremis March 22, 2019 23:03
luisremis
luisremis previously approved these changes Mar 27, 2019
Copy link
Contributor

@luisremis luisremis left a comment

Choose a reason for hiding this comment

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

lgtm! I would change the name of the test tho. "EntityBug" does not tell anything.

@luisremis
Copy link
Contributor

Also, it would be great to have python tests for this kind of bug fixes, even if similar to the unit tests you have for c++

@vishakha041
Copy link
Contributor Author

Do you want me to merge this fix before the VCL changes or after?

@luisremis
Copy link
Contributor

Looks good to go! Conflicts should be straight forward to fix. Either adding and extra commit for the conflict fix or amending the last commit (with the tests cases) should be fine.

If no _ref is used, an empty search result is fine.
However, there is still an error if no results are found
using the given constraints and there is a _ref value used.
@vishakha041
Copy link
Contributor Author

Should be good to merge now. Take a look. Passes on my system with the new test hierarchy.

luisremis
luisremis previously approved these changes Mar 29, 2019
Copy link
Contributor

@luisremis luisremis left a comment

Choose a reason for hiding this comment

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

lgtm!

@vishakha041 vishakha041 merged commit 45f651f into develop Mar 29, 2019
@luisremis luisremis deleted the bug-fixes branch May 3, 2019 23:15
cwlacewe added a commit to cwlacewe/vdms that referenced this pull request Apr 14, 2023
* Format CMakeLists.txt

* Add CSV CPP Client Plugin

* Fix the VDMS path dependencies in csv_plugin library

* Add the multithread layer class in CSVParser class that uses the utilities in CSVPArserUtil.cpp and add the test in tests folder

* merge csv plugin in cleint folder

* Update check-in Dockerfiles to include lib

* Add the csv-plugin as CSVParser in the VDMS::Client CPP folder as a utility

* Remove COpy csv-cpp-lib

* fix the failing tests

* adjust run python_tets

* fix the stash conflict

* wprking on csv tests

* Working os csv tests

* Update run_coverage_cpp.sh

Include .cpp files

* Update run_coverage_cpp.sh

Add flag to overcome known error (https://gcovr.com/en/master/guide/gcov_parser.html#negative-hit-counts)

* stash changes

* Configue the prot of csv parser

* Trying to add the video tesing in csv

* Fi the test script

* fix run_python_test

* Uncoment the Update in CSV

* Csv client lib (IntelLabs#95)

* Update python coverage test to exclude tests folder

* Remove unused old folder (csv-cpp-lib), remove hardcoded ragaad paths from CMakeLists files, remove commented code

* Fix splitrow, add bin to supported image format, uncomment constraints for images, use CSVformat100.csv for entity test, add operations to image and video csv

* Update tests/unit_tests/client_add_entity.cc

Removed empty line (only change to file)

* Update CMakeLists.txt

Remove empty line (only change to file)

* Update tests/main.cc

remove empty line

* Csv client lib (IntelLabs#96)

* Fix ops in video csv, update csvs for additional testing, improve isInt

* Add additional tests and catch case for unkonwn commandtype

* Update tests/main.cc

---------

Co-authored-by: Lacewell, Chaunte W <[email protected]>
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