Skip to content

Conversation

@IAL32
Copy link
Contributor

@IAL32 IAL32 commented Dec 6, 2022

Related: #28039 #28139 #28145 @Taragolis

Migrate Amazon provider's transfer tests to pytest.

All changes are more or less straightforward:

  • Get rid of unittests.TestCase class and TestCase.assert* methods
  • Convert setUp* and tearDown* methods to appropriate pytest alternative
  • Replace decorator parameterized.expand by pytest.mark.parametrize.
  • Renamed @patch to @mock.patch to conform to other tests

@IAL32 IAL32 requested a review from eladkal as a code owner December 6, 2022 11:43
@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Dec 6, 2022
Comment on lines +46 to +55
CREATE_TRANSFORM_PARAMS_INTEGER_FIELDS: dict = {
"TransformJobName": "job_name",
"ModelName": "model_name",
"MaxConcurrentTransforms": 12,
"MaxPayloadInMB": 6,
"BatchStrategy": "MultiRecord",
"TransformInput": {"DataSource": {"S3DataSource": {"S3DataType": "S3Prefix", "S3Uri": "s3_uri"}}},
"TransformOutput": {"S3OutputPath": "output_path"},
"TransformResources": {"InstanceType": "ml.m4.xlarge", "InstanceCount": 3},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: previous implementation relied on the fact that executing the operator actively modified the CREATE_TRANSFORM_PARAMS constant, which is bad practice. Here I provide a second dictionary with the expected integer values and modified tests accordingly.

@IAL32
Copy link
Contributor Author

IAL32 commented Dec 7, 2022

@Taragolis all done!

@potiuk potiuk merged commit b002215 into apache:main Dec 8, 2022
@IAL32 IAL32 deleted the ac/migrate-amzn-operators-pytest branch December 8, 2022 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants