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

[RFR] Update pre-commit hooks rev #10029

Merged
merged 2 commits into from
Apr 1, 2020

Conversation

digitronik
Copy link
Contributor

@digitronik digitronik commented Apr 1, 2020

Purpose or Intent

NOTE: Changes are related to pre-commit run :D (not maual)

PRT Run

@RonnyPfannschmidt
Copy link
Contributor

all of those fauxfactory.gen_$name that moved into f strings might help readability if from fauxfactory import gen_$name as faux_name was used
but that convention also has some downs, it may not be worth it

@@ -73,7 +73,7 @@ def candu_db_restore(temp_appliance_extended_db):
# get DB backup file
db_storage_hostname = conf.cfme_data.bottlenecks.hostname
db_storage_ssh = SSHClient(hostname=db_storage_hostname, **conf.credentials.bottlenecks)
rand_filename = "/tmp/db.backup_{}".format(fauxfactory.gen_alphanumeric())
rand_filename = f"/tmp/db.backup_{fauxfactory.gen_alphanumeric()}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
rand_filename = f"/tmp/db.backup_{fauxfactory.gen_alphanumeric()}"
rand_filename = fauxfactory.gen_alphanumeric(start="/tmp/db.backup_", length=20)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you're going to implement this then same change is required everywhere in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@valaparthvi this change more related to pyupgrade. we are adding this in support of last changes. We will do in another PR; I'm avoiding this as inderectly fauxfactory also using .format

cfme/tests/cloud/test_quota_tagging.py Outdated Show resolved Hide resolved
cfme/tests/cloud/test_quota_tagging.py Outdated Show resolved Hide resolved
cfme/utils/smem_memory_monitor.py Outdated Show resolved Hide resolved
@digitronik
Copy link
Contributor Author

digitronik commented Apr 1, 2020

all of those fauxfactory.gen_$name that moved into f strings might help readability if from fauxfactory import gen_$name as faux_name was used

yup

but that convention also has some downs, it may not be worth it

I added prefix like start in fauxfactory to avoid .format (mainly py2.7) support :D. But now we moved to py3.7 so that f-string doing this work.

I agreed with you... need to think about more readability.

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 :)

cfme/fixtures/candu.py Show resolved Hide resolved
cfme/tests/optimize/test_bottlenecks.py Show resolved Hide resolved
@dajoRH
Copy link
Contributor

dajoRH commented Apr 1, 2020

I detected some fixture changes in commit dcb6aee

Show fixtures

The global fixture candu_db_restore is used in the following files:

  • cfme/tests/candu/test_graph_groupbytag.py
    • test_tagwise

The global fixture depot_machine_ipv4_and_ipv6 is used in the following files:

  • cfme/tests/configure/test_log_depot_operation.py
    • test_log_collection_over_ipv6

The global fixture import_dialog is used in the following files:

  • cfme/tests/automate/test_service_automate.py
  • cfme/tests/automate/test_service_dialog.py
    • custom_button
  • cfme/tests/services/test_dialog_element_in_catalog.py
    • test_request_details_page_tagcontrol_field
    • test_save_dynamic_multi_drop_down_dialog
  • cfme/tests/services/test_dynamicdd_dialogelement.py
    • test_dynamic_submit_cancel_button_service
  • cfme/tests/services/test_myservice.py
    • test_load_service_with_button
  • cfme/tests/services/test_rest_services.py
  • cfme/tests/services/test_service_customer_bz.py
    • test_edit_import_dialog

The local fixture provider_credentials is used in the following files:

  • cfme/tests/ansible/test_embedded_ansible_services.py

The local fixture default_alerts is used in the following files:

  • cfme/tests/control/test_default_alerts.py
    • test_default_alerts

The local fixture vm_ownership is used in the following files:

  • cfme/tests/intelligence/chargeback/test_chargeback_long_term_rates.py
    • resource_usage
    • chargeback_report_custom

The local fixture db_restore is used in the following files:

  • cfme/tests/optimize/test_bottlenecks.py
    • test_bottlenecks_report_event_groups
    • test_bottlenecks_report_show_host_events
    • test_bottlenecks_report_time_zone
    • test_bottlenecks_summary_event_groups
    • test_bottlenecks_summary_show_host_events
    • test_bottlenecks_summary_time_zone

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

@mshriver mshriver merged commit 9ce2cc7 into ManageIQ:master Apr 1, 2020
ganeshhubale pushed a commit to ganeshhubale/integration_tests that referenced this pull request Jul 21, 2020
* Update pre-commit hooks rev

* some pyupgrade changes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants