Refactor unit tests for codec #562
Conversation
Signed-off-by: Martin Gaievski <[email protected]>
martin-gaievski
added
Refactoring
Codecov Report
@@ Coverage Diff @@
## main #562 +/- ##
============================================
- Coverage 84.07% 84.05% -0.03%
+ Complexity 1021 1020 -1
============================================
Files 146 146
Lines 4195 4195
Branches 373 373
============================================
- Hits 3527 3526 -1
- Misses 492 493 +1
Partials 176 176
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| reader.close(); | ||
|
|
||
| /** | ||
| * Add doc with field "my_vector" |
There was a problem hiding this comment.
this is not my_vector anymore.
| iwc.setCodec(codec); | ||
|
|
||
| /** | ||
| * Add doc with field "test_vector" |
There was a problem hiding this comment.
this should be test_vector_one?
| final MapperService mapperService = mock(MapperService.class); | ||
| final KNNMethodContext knnMethodContext = new KNNMethodContext( | ||
| KNNEngine.LUCENE, | ||
| SpaceType.L2, | ||
| new MethodComponentContext(METHOD_HNSW, Map.of(HNSW_ALGO_M, 16, HNSW_ALGO_EF_CONSTRUCTION, 256)) | ||
| ); | ||
| final KNNVectorFieldMapper.KNNVectorFieldType mappedFieldType1 = new KNNVectorFieldMapper.KNNVectorFieldType( | ||
| fieldName, | ||
| FIELD_NAME_ONE, |
There was a problem hiding this comment.
Will there be a way for this class doesn't need to know about FIELD_NAME_ONE and FIELD_NAME_TWO?
This code might reduce code duplication but it seems it does not achieve a separation of concerns. I had to look in to testKnnVectorIndex code and what it does and how FIELD_NAME_ONE and FIELD_NAME_TWO is used. Maybe, it could be because I am not familiar with KNN code base though.
There was a problem hiding this comment.
I see what you mean, I'm also not that happy with this change. let me try few things.
There was a problem hiding this comment.
I think I've got it with a function as supplier, will push the commit soon
…class Signed-off-by: Martin Gaievski <[email protected]>
Signed-off-by: Martin Gaievski <[email protected]>
* Refactor unit test for codec for easier lucene version upgrades Signed-off-by: Martin Gaievski <[email protected]> (cherry picked from commit 3201c3a)
* Refactor unit test for codec for easier lucene version upgrades Signed-off-by: Martin Gaievski <[email protected]>
* Refactor unit test for codec for easier lucene version upgrades Signed-off-by: Martin Gaievski <[email protected]>
heemin32
added
v2.4.0
Signed-off-by: Martin Gaievski [email protected]
Description
Current structure of unit tests for codec does not allow to upgrade codec easy. For new codec it's required to copy test code to a new codec specific unit test. With this change this test has been parametrized and codec and perFieldFormat can be injected by caller.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.