Skip to content

Conversation

@sararob
Copy link
Contributor

@sararob sararob commented May 25, 2022

Refactored test_training_jobs.py to include a temp directory for the test Python script file. This will allow the unit tests to run on different environments.

@product-auto-label product-auto-label bot added size: s Pull request size is small. api: vertex-ai Issues related to the googleapis/python-aiplatform API. labels May 25, 2022
Comment on lines +77 to +78
_TEST_TEMPDIR = tempfile.mkdtemp()
_TEST_LOCAL_SCRIPT_FILE_PATH = os.path.join(_TEST_TEMPDIR, _TEST_LOCAL_SCRIPT_FILE_NAME)
Copy link
Contributor

@ucdmkt ucdmkt May 25, 2022

Choose a reason for hiding this comment

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

Do you think you can afford dependency on absl-py? If so, standardizing on tmp dir handling, with ability to inject via a public envvar, is beneficial.

https://github.com/abseil/abseil-py/blob/main/absl/testing/absltest.py#L158-L165

Alternatively, pytest also has a built in fixture to do so in a standard way , but I understand that pytest is used only for test driver, but not as a unittest utility framework as it is now, so it will be anyways additional dependency. I misspoke on this one. pytest is also used as utility.

https://docs.pytest.org/en/6.2.x/tmpdir.html#base-temporary-directory

Copy link
Contributor

@jaycee-li jaycee-li left a comment

Choose a reason for hiding this comment

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

LGTM

@sararob sararob merged commit 74dbabd into googleapis:main May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: vertex-ai Issues related to the googleapis/python-aiplatform API. size: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants