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

[1LP][RFR] NTP settings fixes #10144

Merged
merged 1 commit into from
Jun 5, 2020
Merged

[1LP][RFR] NTP settings fixes #10144

merged 1 commit into from
Jun 5, 2020

Conversation

tpapaioa
Copy link
Contributor

@tpapaioa tpapaioa commented May 26, 2020

Made a few changes to the NTP server tests. The main changes are:

  • Changed the test finalizers to make sure the server-level NTP servers are set to the ones defined in cfme_data, instead of being cleared out. If the server-level settings are cleared out, then the zone-level defaults are used, and those public NTP server defaults are not the ones we want to use.
  • Moved NTP server dictionaries for empty / config / random / max char length values out of the tests and into fixtures.

{{ pytest: -vv cfme/tests/configure/test_ntp_server.py }}

@dajoRH
Copy link
Contributor

dajoRH commented May 26, 2020

I detected some fixture changes in commit a660c4c

The local fixture ntp_server_keys is used in the following files:

  • cfme/tests/configure/test_ntp_server.py
    • empty_ntp
    • random_ntp
    • random_max_ntp
    • config_ntp

The local fixture empty_ntp is used in the following files:

  • cfme/tests/configure/test_ntp_server.py
    • clear_ntp_settings
    • test_ntp_crud

The local fixture random_ntp is used in the following files:

  • cfme/tests/configure/test_ntp_server.py
    • test_ntp_crud

The local fixture random_max_ntp is used in the following files:

  • cfme/tests/configure/test_ntp_server.py
    • test_ntp_server_max_character

The local fixture config_ntp is used in the following files:

  • cfme/tests/configure/test_ntp_server.py
    • test_ntp_crud
    • test_ntp_server_max_character

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

@tpapaioa tpapaioa changed the title [WIPTEST] NTP settings fixes [RFR] NTP settings fixes May 28, 2020
Copy link
Contributor

@john-dupuy john-dupuy left a comment

Choose a reason for hiding this comment

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

Nice PR @tpapaioa LGTM!

@john-dupuy john-dupuy changed the title [RFR] NTP settings fixes [1LP][RFR] NTP settings fixes Jun 3, 2020
@mshriver mshriver self-assigned this Jun 5, 2020
@mshriver mshriver merged commit edf2bf6 into ManageIQ:master Jun 5, 2020
@tpapaioa tpapaioa deleted the fix_test_ntp_conf_file_update_check branch June 5, 2020 15:02
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.

4 participants