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.

Sign up for GitHub

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

Jump to bottom

Updated Testcases for MLEngine.java #1536

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Updated Testcases for MLEngine.java #1536

wants to merge 4 commits into from

Conversation

divitr
Copy link
Contributor

@divitr divitr commented Oct 23, 2023

Description

Updated Testcases for MLEngine.java

Issues Resolved

Resolves #1376

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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.

@divitr divitr had a problem deploying to ml-commons-cicd-env October 23, 2023 01:17 — with GitHub Actions Failure
@divitr divitr had a problem deploying to ml-commons-cicd-env October 23, 2023 01:17 — with GitHub Actions Failure
@dhrubo-os
Copy link
Collaborator

Tests are failing, could you please check?

@divitr
updated setUp
73b8545 
Signed-off-by: Divit Rawal <[email protected]>
@divitr divitr temporarily deployed to ml-commons-cicd-env October 23, 2023 23:14 — with GitHub Actions Inactive
@divitr divitr had a problem deploying to ml-commons-cicd-env October 23, 2023 23:14 — with GitHub Actions Failure
@divitr divitr temporarily deployed to ml-commons-cicd-env October 23, 2023 23:14 — with GitHub Actions Inactive
@divitr divitr temporarily deployed to ml-commons-cicd-env October 23, 2023 23:14 — with GitHub Actions Inactive
@divitr divitr had a problem deploying to ml-commons-cicd-env October 23, 2023 23:42 — with GitHub Actions Failure
@divitr divitr temporarily deployed to ml-commons-cicd-env October 23, 2023 23:42 — with GitHub Actions Inactive
@divitr divitr temporarily deployed to ml-commons-cicd-env October 23, 2023 23:42 — with GitHub Actions Inactive
@divitr divitr had a problem deploying to ml-commons-cicd-env October 29, 2023 23:08 — with GitHub Actions Failure
@divitr
Copy link
Contributor Author

divitr commented Oct 30, 2023 •
edited
Loading

Hmm, all tests seemed to be working correctly on my machine, I'll take a look.

Just built, it says all tests in MLEngineTest.java are passing.
Screenshot 2023-10-23 at 5 24 01 PM

Well, I believe this is failing in windows test. May be you can try to print both of the values and check why this is different in windows?

Should be fixed now!

@divitr divitr temporarily deployed to ml-commons-cicd-env October 30, 2023 16:12 — with GitHub Actions Inactive
@divitr divitr had a problem deploying to ml-commons-cicd-env October 30, 2023 16:12 — with GitHub Actions Failure
@divitr divitr had a problem deploying to ml-commons-cicd-env October 30, 2023 16:12 — with GitHub Actions Failure
@divitr divitr had a problem deploying to ml-commons-cicd-env October 31, 2023 21:40 — with GitHub Actions Failure
@divitr divitr temporarily deployed to ml-commons-cicd-env October 31, 2023 21:40 — with GitHub Actions Inactive
@divitr divitr temporarily deployed to ml-commons-cicd-env October 31, 2023 21:40 — with GitHub Actions Inactive
@divitr divitr temporarily deployed to ml-commons-cicd-env November 5, 2023 16:18 — with GitHub Actions Inactive
@divitr divitr temporarily deployed to ml-commons-cicd-env November 5, 2023 16:18 — with GitHub Actions Inactive
@divitr divitr temporarily deployed to ml-commons-cicd-env November 5, 2023 16:18 — with GitHub Actions Inactive
@dhrubo-os
Copy link
Collaborator

Could you please add tests for deployExecute & encrypt method?

@dhrubo-os
Copy link
Collaborator

@divitr Could you please resolve the merge conflict?

@dhrubo-os
Copy link
Collaborator

@divitr Are you working on this?

@austintlee
Copy link
Collaborator

Should we close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zhangxunmt Zhangxunmt Zhangxunmt approved these changes

@b4sjoo b4sjoo Awaiting requested review from b4sjoo b4sjoo is a code owner

@jngz-es jngz-es Awaiting requested review from jngz-es jngz-es is a code owner

@model-collapse model-collapse Awaiting requested review from model-collapse model-collapse is a code owner

@rbhavna rbhavna Awaiting requested review from rbhavna rbhavna is a code owner

@wujunshen wujunshen Awaiting requested review from wujunshen

@ylwu-amzn ylwu-amzn Awaiting requested review from ylwu-amzn ylwu-amzn is a code owner

@zane-neo zane-neo Awaiting requested review from zane-neo zane-neo is a code owner

@dhrubo-os dhrubo-os Awaiting requested review from dhrubo-os dhrubo-os is a code owner

@austintlee austintlee Awaiting requested review from austintlee austintlee is a code owner

@HenryL27 HenryL27 Awaiting requested review from HenryL27 HenryL27 is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Improve test coverage for MLEngine Class
4 participants
@divitr @dhrubo-os @austintlee @Zhangxunmt