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

test(grpc): e2e + unit tests #2984

Merged
merged 18 commits into from Sep 29, 2022
Merged

test(grpc): e2e + unit tests #2984

merged 18 commits into from Sep 29, 2022

Conversation

aarnphm
Copy link
Member

@aarnphm aarnphm commented Sep 12, 2022

What does this PR address?

This PR is a follow up on #2808 for tests related manners

This includes e2e, unit tests

This will also include a fix in our e2e folder structure where test model should not be
saved under bentoml local model directory if users run tests locally.

This PR also fixes multipart handles for from_proto and to_proto.

depends on #3012

@aarnphm aarnphm requested review from ssheng, parano and a team as code owners September 12, 2022 21:57
@aarnphm aarnphm requested review from sauyon and removed request for a team September 12, 2022 21:57
@aarnphm aarnphm changed the base branch from main to grpc September 12, 2022 21:58
aarnphm added a commit to aarnphm/BentoML that referenced this pull request Sep 12, 2022
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
@aarnphm aarnphm mentioned this pull request Sep 12, 2022
3 tasks
@codecov
Copy link

codecov bot commented Sep 12, 2022

Codecov Report

Merging #2984 (da8df4b) into main (b3bd5a7) will increase coverage by 8.45%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2984      +/-   ##
==========================================
+ Coverage   61.22%   69.68%   +8.45%     
==========================================
  Files         132      115      -17     
  Lines       11521    10766     -755     
  Branches     1924     1834      -90     
==========================================
+ Hits         7054     7502     +448     
+ Misses       4035     2826    -1209     
- Partials      432      438       +6     
Impacted Files Coverage Δ
bentoml/_internal/io_descriptors/file.py 84.69% <ø> (+21.42%) ⬆️
bentoml/_internal/io_descriptors/image.py 76.66% <ø> (+24.66%) ⬆️
bentoml/grpc/utils/__init__.py 88.88% <50.00%> (+38.20%) ⬆️
bentoml/models.py 77.04% <50.00%> (ø)
bentoml/_internal/server/grpc/servicer.py 61.72% <66.66%> (+61.72%) ⬆️
bentoml/_internal/io_descriptors/multipart.py 94.11% <100.00%> (+26.47%) ⬆️
bentoml/picklable_model.py 0.00% <0.00%> (-100.00%) ⬇️
bentoml/exceptions.py 6.25% <0.00%> (-87.50%) ⬇️
bentoml/_internal/utils/uri.py 68.42% <0.00%> (-10.53%) ⬇️
bentoml/_internal/utils/buildx.py 19.04% <0.00%> (-5.45%) ⬇️
... and 53 more

@aarnphm aarnphm force-pushed the test/grpc branch 3 times, most recently from 4d635b3 to c024a53 Compare September 27, 2022 19:55
ssheng pushed a commit that referenced this pull request Sep 27, 2022
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>

## What does this PR address?

<!--
Thanks for sending a pull request!

Congrats for making it this far! Here's a 🍱 for you. There are still a
few steps ahead.

Please make sure to read the contribution guidelines, then fill out the
blanks below before requesting a code review.

Name your Pull Request with one of the following prefixes, e.g. "feat:
add support for PyTorch", to indicate the type of changes proposed. This
is based on the [Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/#summary).
  - feat: (new feature for the user, not a new feature for build script)
  - fix: (bug fix for the user, not a fix to a build script)
  - docs: (changes to the documentation)
- style: (formatting, missing semicolons, etc; no production code
change)
  - refactor: (refactoring production code, eg. renaming a variable)
  - perf: (code changes that improve performance)
- test: (adding missing tests, refactoring tests; no production code
change)
  - chore: (updating grunt tasks etc; no production code change)
- build: (changes that affect the build system or external dependencies)
  - ci: (changes to configuration files and scripts)
  - revert: (reverts a previous commit)

Describe your changes in detail. Attach screenshots here if appropriate.

Once you're done with this, someone from BentoML team or community
member will help review your PR (see "Who can help review?" section for
potential reviewers.). If no one has reviewed your PR after a week have
passed, don't hesitate to post a new comment and ping @-the same person.
Notifications sometimes get lost 🥲.
-->

<!-- Remove if not applicable -->

Fixes #2991 
Depends on #2984 


## Before submitting:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!--- If you plan to update documentation or tests in follow-up, please
note -->

- [ ] Does the Pull Request follow [Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/#summary)
naming? Here are [GitHub's

guide](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request)
on how to create a pull request.
- [x] Does the code follow BentoML's code style, both `make format` and
`make lint` script have passed
([instructions](https://github.com/bentoml/BentoML/blob/main/DEVELOPMENT.md#style-check-auto-formatting-type-checking))?
- [x] Did you read through [contribution
guidelines](https://github.com/bentoml/BentoML/blob/main/CONTRIBUTING.md#ways-to-contribute)
and follow [development
guidelines](https://github.com/bentoml/BentoML/blob/main/DEVELOPMENT.md#start-developing)?
- [ ] Did your changes require updates to the documentation? Have you
updated
those accordingly? Here are [documentation
guidelines](https://github.com/bentoml/BentoML/tree/main/docs) and [tips
on writting
docs](https://github.com/bentoml/BentoML/tree/main/docs#writing-documentation).
- [ ] Did you write tests to cover your changes?

Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
@@ -121,7 +121,7 @@ message Part {

// Series portrays a series of values. This can be used for
// representing Series types in tabular data.
Series series = 5;
Series series =5;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrevert?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh how did it got here :(

Copy link
Member Author

Choose a reason for hiding this comment

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

I think i address this in one of the PR

@aarnphm aarnphm changed the title EXPERIMENTAL[grpc]: e2e and unit tests test(grpc): e2e + unit tests Sep 29, 2022
@aarnphm aarnphm enabled auto-merge (squash) September 29, 2022 02:23
@aarnphm aarnphm merged commit 8ccf0a2 into bentoml:main Sep 29, 2022
@aarnphm aarnphm deleted the test/grpc branch September 29, 2022 02:34
@bojiang bojiang mentioned this pull request Oct 12, 2022
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.

ci: broken CI
4 participants