ByronHsu commented on a change in pull request #597:
URL: https://github.com/apache/submarine/pull/597#discussion_r644019766
##########
File path: submarine-sdk/pysubmarine/tests/models/test_model.py
##########
@@ -33,10 +36,16 @@ def tearDown(self):
pass
@pytest.mark.skip(reason="Developing")
- def test_log_model(self, mocker):
+ def test_log_model(self, mocker, package_type):
Review comment:
This test is problematic. `package_type` would be None. You should
separate different package types into different unit tests.
##########
File path: submarine-sdk/pysubmarine/tests/models/test_model_e2e.py
##########
@@ -35,7 +35,7 @@ def models_client_fixture():
class TestSubmarineModelsClientE2E():
def test_model(self, models_client):
- model = LinearNNModel()
+ model = LinearNNModelTorch()
Review comment:
Same problem. Should test all packages.
##########
File path: submarine-sdk/pysubmarine/tests/models/test_model.py
##########
@@ -54,7 +63,7 @@ def test_update_model(self, mocker):
def test_load_model(self, mocker):
mock_method = mocker.patch.object(mlflow.pyfunc, "load_model")
mock_method.return_value = mlflow.pytorch._PyTorchWrapper(
- LinearNNModel())
+ LinearNNModelTorch())
Review comment:
Why only test pytorch?
##########
File path: submarine-sdk/pysubmarine/tests/models/test_model.py
##########
@@ -33,10 +36,16 @@ def tearDown(self):
pass
@pytest.mark.skip(reason="Developing")
Review comment:
This line should be removed, or it will not run this test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
