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

fix: test expects file with contents #2221

Merged
merged 1 commit into from
Mar 24, 2021
Merged

fix: test expects file with contents #2221

merged 1 commit into from
Mar 24, 2021

Conversation

cristianmtr
Copy link
Contributor

@cristianmtr cristianmtr commented Mar 22, 2021

Test expects all .py files in its dir to have contents. Ensure they all do, and ensure it always checks the same files, no matter where you're running it from

@cristianmtr cristianmtr requested a review from a team as a code owner March 22, 2021 11:36
@jina-bot jina-bot added size/XS area/testing This issue/PR affects testing labels Mar 22, 2021
@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #2221 (e208118) into master (e0edf3b) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2221      +/-   ##
==========================================
- Coverage   90.43%   90.42%   -0.01%     
==========================================
  Files         208      208              
  Lines       11253    11253              
==========================================
- Hits        10177    10176       -1     
- Misses       1076     1077       +1     
Flag Coverage Δ
daemon 51.25% <100.00%> (ø)
jina 90.88% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/__init__.py 73.49% <100.00%> (ø)
jina/drivers/querylang/select.py 82.75% <0.00%> (-6.90%) ⬇️
jina/docker/hubapi/local.py 89.01% <0.00%> (-1.10%) ⬇️
jina/peapods/runtimes/zmq/zed.py 92.90% <0.00%> (+1.41%) ⬆️

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 576a3c8...e208118. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Mar 22, 2021

Latency summary

Current PR yields:

  • 😶 index QPS at 964, delta to last 3 avg.: +0%
  • 😶 query QPS at 14, delta to last 3 avg.: +0%

Breakdown

Version Index QPS Query QPS
current 964 14
1.0.15 961 14
1.0.14 958 13

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

@@ -409,9 +409,9 @@ def validate(req):
f = Flow(restful=restful, read_only=True).add(
uses=os.path.join(cur_dir, '../yaml/datauriindex.yml'), timeout_ready=-1
)

files = os.path.join(cur_dir, '*.py')
Copy link
Member

Choose a reason for hiding this comment

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

interesting that we use python files as text input.
Wouldn't it be better to create files in the tmpdir instead of reading from the program code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can just use the yaml files. We know these will always have content

Copy link
Member

@nan-wang nan-wang left a comment

Choose a reason for hiding this comment

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

LGTM👍

@nan-wang
Copy link
Member

good catch!

@nan-wang nan-wang merged commit 9b3ab47 into master Mar 24, 2021
@nan-wang nan-wang deleted the fix-index-files branch March 24, 2021 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing This issue/PR affects testing size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants