Skip to content

Conversation

@IAL32
Copy link
Contributor

@IAL32 IAL32 commented Dec 5, 2022

Related: #28039 @Taragolis

Migrate Amazon provider's sensors and utils 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.

@IAL32 IAL32 requested a review from eladkal as a code owner December 5, 2022 23:58
@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Dec 5, 2022
@Taragolis
Copy link
Contributor

Nice 👍

Could you also replace parameterized.expand by pytest.mark.parametrize it is more powerful, some example. You could find all modules where parameterized.expand uses by run grep -rl 'parameterized' ./tests/providers/amazon/aws/sensors | uniq

@Taragolis
Copy link
Contributor

cc: @vincbeck @vandonr-amz

Co-authored-by: Andrey Anshin <Andrey.Anshin@taragol.is>
@Taragolis Taragolis requested a review from ferruzzi December 6, 2022 12:56
Copy link
Contributor

@Taragolis Taragolis left a comment

Choose a reason for hiding this comment

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

For me looking good. But might be @ferruzzi @vandonr-amz @o-nikolas @vincbeck have some comments

Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

LGTM

@potiuk potiuk merged commit b726d8e into apache:main Dec 6, 2022
Copy link
Contributor

@ferruzzi ferruzzi left a comment

Choose a reason for hiding this comment

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

Left one nitpick, but looks good. Thank you both for doing these, it's something I've been thinking about for a while

def test_poke_on_success_state(self, mock_get_job_description):
mock_get_job_description.return_value = {"status": "SUCCEEDED"}
self.assertTrue(self.batch_sensor.poke({}))
assert self.batch_sensor.poke({})
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking suggestion: missed one is True here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahhh dang.
I included this change here: 9960d43

Copy link
Contributor

Choose a reason for hiding this comment

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

Jarek sniped me on the merge, but it's all good. It's a minor thing.

Taragolis pushed a commit that referenced this pull request Dec 7, 2022
…28145)

Related: #28039 #28139

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 deleted the ac/migrate-amz-sensor-tests branch December 8, 2022 07:03
ephraimbuddy pushed a commit that referenced this pull request Mar 8, 2023
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.

6 participants