test: refactor driver test #1452

bwanglzu · 2020-12-13T13:14:42Z

minor test refactoring on the querylang and driver

JoanFM · 2020-12-13T13:21:40Z

tests/unit/drivers/test_matches2doc_rank_drivers.py

@@ -41,17 +41,15 @@ def create_document_to_score():
    # |- matches: (id: 3, parent_id: 1, score.value: 3),
    # |- matches: (id: 4, parent_id: 1, score.value: 4),
    # |- matches: (id: 5, parent_id: 1, score.value: 5),
-
-    doc = jina_pb2.DocumentProto()
+    doc = Document()


the idea herr is to see if we can refactor the logic of the test itself to be less dependant on id? I am not sure it is possible though

One problem is that this test is so dependant on document logic that any document refactor involves a lot of effort to regfactor these tests. is there a way to avoid this?

the option I can see is to maintain fixtures in a separate file so that we can reuse/modify it anywhere

butis there so many shared code?

there could be, if we "composite" small fixtures into bigger ones, a lot of code are actually shared, from low levels.

okey that'd be great.

github-actions · 2020-12-13T13:28:30Z

Latency summary

Current PR yields:

😶 index QPS at 2186, delta to last 3 avg.: -2%
🐎🐎 query QPS at 41, delta to last 3 avg.: +10%

Breakdown

Version	Index QPS	Query QPS
current	2186	41
`0.8.7`	2254	37
`0.8.6`	2253	37
`0.8.5`	2245	35

Backed by latency-tracking. Further commits will update this comment.

tests/unit/drivers/querylang/queryset/test_lookup.py

tests/integration/hub_usage/dummyhub_abs/__init__.py

JoanFM · 2020-12-14T21:58:23Z

tests/unit/drivers/test_matches2doc_rank_drivers.py

@@ -41,17 +41,15 @@ def create_document_to_score():
    # |- matches: (id: 3, parent_id: 1, score.value: 3),
    # |- matches: (id: 4, parent_id: 1, score.value: 4),
    # |- matches: (id: 5, parent_id: 1, score.value: 5),
-
-    doc = jina_pb2.DocumentProto()
+    doc = Document()


okey that'd be great.

bwanglzu · 2020-12-15T23:22:17Z

related to #975 , hubio test both locally and in the ci.

bwanglzu · 2020-12-15T23:26:28Z

local test passed after running docker build -f Dockerfiles/pip.Dockerfile -t jinaai/jina:test-pip .

nan-wang

minor changes

nan-wang · 2020-12-16T03:59:31Z

tests/unit/drivers/querylang/queryset/test_lookup.py

+    assert len(filtered_docs) == 1
+    for d in filtered_docs:
+        assert int(d.tags['id']) == 4
+        assert len(d.chunks) == 5


Is there any specific reason to check the number of chunks here?

Suggested change

assert len(d.chunks) == 5

assert len(d.chunks) == 5

codecov · 2020-12-16T12:40:12Z

Codecov Report

Merging #1452 (1a33145) into master (08fa53e) will increase coverage by 0.24%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1452      +/-   ##
==========================================
+ Coverage   82.76%   83.00%   +0.24%     
==========================================
  Files         110      110              
  Lines        6381     6443      +62     
==========================================
+ Hits         5281     5348      +67     
+ Misses       1100     1095       -5

Impacted Files	Coverage Δ
jina/drivers/querylang/queryset/helper.py	`92.30% <100.00%> (-0.55%)`	⬇️
jina/excepts.py	`100.00% <100.00%> (ø)`
jina/logging/profile.py	`68.33% <0.00%> (-0.58%)`	⬇️
jina/drivers/craft.py	`100.00% <0.00%> (ø)`
jina/types/ndarray/generic.py	`100.00% <0.00%> (ø)`
jina/executors/indexers/cache.py	`100.00% <0.00%> (ø)`
jina/executors/evaluators/text/length.py	`100.00% <0.00%> (ø)`
jina/executors/evaluators/rank/precision.py	`100.00% <0.00%> (ø)`
jina/peapods/peas/__init__.py	`90.72% <0.00%> (+0.03%)`	⬆️
jina/drivers/evaluate.py	`98.21% <0.00%> (+0.03%)`	⬆️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 771d9bc...5e0ee4c. Read the comment docs.

bwanglzu · 2020-12-17T09:51:48Z

@nan-wang sorry I was off yesterday, just saw your message. The test was actually copied from another file test_queryset.py, I merged test_queryset into test_lookup since they're in the same file in our codebase. I think the objective of the test is just to double check number of chunks.

test: refactor driver

1df8dd3

bwanglzu self-assigned this Dec 13, 2020

jina-bot added size/M area/core This issue/PR affects the core codebase area/helper This issue/PR affects the helper functionality area/testing This issue/PR affects testing component/driver labels Dec 13, 2020

bwanglzu marked this pull request as ready for review December 13, 2020 13:17

bwanglzu requested a review from a team as a code owner December 13, 2020 13:17

bwanglzu requested review from nan-wang and florian-hoenicke December 13, 2020 13:17

JoanFM reviewed Dec 13, 2020

View reviewed changes

bwanglzu marked this pull request as draft December 13, 2020 20:56

florian-hoenicke requested changes Dec 13, 2020

View reviewed changes

tests/unit/drivers/querylang/queryset/test_lookup.py Outdated Show resolved Hide resolved

tests/unit/drivers/querylang/queryset/test_lookup.py Outdated Show resolved Hide resolved

bwanglzu changed the title ~~test: refactor driver~~ test: refactor driver test Dec 14, 2020

test: refactor driver test

e1551b4

JoanFM reviewed Dec 14, 2020

View reviewed changes

bwanglzu added 3 commits December 14, 2020 23:00

test: refactor driver test

d006d99

test: refactor driver test

5c2b305

Merge branch 'master' into test-refactor-querylang

440cd0f

bwanglzu marked this pull request as ready for review December 14, 2020 22:43

florian-hoenicke approved these changes Dec 14, 2020

View reviewed changes

Merge remote-tracking branch 'origin' into test-refactor-querylang

5e0ee4c

nan-wang requested changes Dec 16, 2020

View reviewed changes

nan-wang approved these changes Dec 17, 2020

View reviewed changes

nan-wang merged commit 2d0c489 into master Dec 17, 2020

nan-wang deleted the test-refactor-querylang branch December 17, 2020 04:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: refactor driver test #1452

test: refactor driver test #1452

bwanglzu commented Dec 13, 2020 •

edited

Loading

JoanFM Dec 13, 2020

JoanFM Dec 13, 2020

bwanglzu Dec 14, 2020

JoanFM Dec 14, 2020

bwanglzu Dec 14, 2020 •

edited

Loading

JoanFM Dec 14, 2020

github-actions bot commented Dec 13, 2020 •

edited

Loading

JoanFM Dec 14, 2020

bwanglzu commented Dec 15, 2020

bwanglzu commented Dec 15, 2020

nan-wang left a comment

nan-wang Dec 16, 2020

codecov bot commented Dec 16, 2020

bwanglzu commented Dec 17, 2020

test: refactor driver test #1452

test: refactor driver test #1452

Conversation

bwanglzu commented Dec 13, 2020 • edited Loading

JoanFM Dec 13, 2020

Choose a reason for hiding this comment

JoanFM Dec 13, 2020

Choose a reason for hiding this comment

bwanglzu Dec 14, 2020

Choose a reason for hiding this comment

JoanFM Dec 14, 2020

Choose a reason for hiding this comment

bwanglzu Dec 14, 2020 • edited Loading

Choose a reason for hiding this comment

JoanFM Dec 14, 2020

Choose a reason for hiding this comment

github-actions bot commented Dec 13, 2020 • edited Loading

Latency summary

Breakdown

JoanFM Dec 14, 2020

Choose a reason for hiding this comment

bwanglzu commented Dec 15, 2020

bwanglzu commented Dec 15, 2020

nan-wang left a comment

Choose a reason for hiding this comment

nan-wang Dec 16, 2020

Choose a reason for hiding this comment

codecov bot commented Dec 16, 2020

Codecov Report

bwanglzu commented Dec 17, 2020

bwanglzu commented Dec 13, 2020 •

edited

Loading

bwanglzu Dec 14, 2020 •

edited

Loading

github-actions bot commented Dec 13, 2020 •

edited

Loading