Skip to content
This repository was archived by the owner on Apr 7, 2022. It is now read-only.

[1LP][RFR] Fix VM retirement tests and test cleanup#10064

Merged
mshriver merged 2 commits intoManageIQ:masterfrom
valaparthvi:retire
May 21, 2020
Merged

[1LP][RFR] Fix VM retirement tests and test cleanup#10064
mshriver merged 2 commits intoManageIQ:masterfrom
valaparthvi:retire

Conversation

@valaparthvi
Copy link
Contributor

@valaparthvi valaparthvi commented Apr 20, 2020

Purpose or Intent

  • Fixing
    1. Tests related to VM retirement
    2. Adding selector to test_add_provider_button_accordion so that it doesn't extra tests.
    3. test_reports_invalid_file which was failing because error message changed.
  • Removing
    1. Blockers from BZs
    2. test_ui_pinning_after_relog and test_ui_notification_icon

PRT Run

{{ pytest: cfme/tests/infrastructure/test_vm_retirement_rest.py cfme/tests/intelligence/reports/test_import_export_reports_widgets.py::test_reports_invalid_file -vvv }}

@valaparthvi valaparthvi changed the title [WIPTEST] Fix VM retirement tests [WIPTEST] Fix VM retirement tests and test cleanup Apr 20, 2020
@valaparthvi valaparthvi added the test-cleanup Test removal, collection changes, re-organization label Apr 20, 2020
@valaparthvi valaparthvi force-pushed the retire branch 9 times, most recently from 925f0e4 to 1261ecd Compare April 27, 2020 09:18
@valaparthvi valaparthvi changed the title [WIPTEST] Fix VM retirement tests and test cleanup [RFR] Fix VM retirement tests and test cleanup Apr 27, 2020
Copy link
Member

Choose a reason for hiding this comment

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

Optional: you can remove BZ from automates.

Copy link
Member

@ganeshhubale ganeshhubale Apr 29, 2020

Choose a reason for hiding this comment

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

Optional: Are you sure about spaces between words?
message = r".*Error during .*upload.*: undefined method `keys.* for .*i.*:String.*"

Can we write like this - message = r".*Error during.*upload.*: undefined method `keys.*for.*i.*:String.*"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think it will be better to leave the space out. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

What are we trying to fix here? using .* is making this expression very greedy, if there's an issue with the number of whitespace characters in the string in the UI, we should use \s+ to express "one or more whitespace characters"

Copy link
Member

@ganeshhubale ganeshhubale left a comment

Choose a reason for hiding this comment

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

LGTM :)

@ganeshhubale ganeshhubale changed the title [RFR] Fix VM retirement tests and test cleanup [1LP][RFR] Fix VM retirement tests and test cleanup Apr 29, 2020
@dajoRH
Copy link
Contributor

dajoRH commented Apr 29, 2020

I detected some fixture changes in commit 28de40c4ac5122bca3db9fde9428fcb964a2b27f

The local fixture retire_action is used in the following files:

  • cfme/tests/infrastructure/test_vm_retirement_rest.py
    • test_retire_vm_now
    • test_retire_vm_future

Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃

@mshriver mshriver changed the title [1LP][RFR] Fix VM retirement tests and test cleanup [1LP][WIPTEST] Fix VM retirement tests and test cleanup May 6, 2020
@valaparthvi valaparthvi changed the title [1LP][WIPTEST] Fix VM retirement tests and test cleanup [1LP][RFR] Fix VM retirement tests and test cleanup May 20, 2020
@mshriver mshriver merged commit 923c1fa into ManageIQ:master May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

fix-test lint-ok test-cleanup Test removal, collection changes, re-organization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants