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

[1LP][RFR] Automate: test_reports_timezone #9904

Merged
merged 2 commits into from
Feb 14, 2020

Conversation

valaparthvi
Copy link
Contributor

@valaparthvi valaparthvi commented Feb 5, 2020

Purpose or Intent

  • Adding tests
    1. test_reports_timezone
  • Enhancement
    1. Enhance cfme.utils.timeutil.parsetime to be compatible with different timezones. This may not work in case the timezone is not found in pytz module.
    2. Add a new property report_timezone to SavedReport entity and modify queued_datetime_in_title and datetime_in_tree accordingly.

PRT Run

{{ pytest: cfme/tests/intelligence/reports/test_reports.py -k "test_reports_timezone" -vvv }}

@valaparthvi valaparthvi added enhancement test-automation To be applied on PR's which are automating existing manual cases labels Feb 5, 2020
@valaparthvi valaparthvi changed the title [WIPTEST] Automate: test_reports_timezone [RFR Automate: test_reports_timezone Feb 5, 2020
@valaparthvi valaparthvi changed the title [RFR Automate: test_reports_timezone [RFR] Automate: test_reports_timezone Feb 5, 2020
@dajoRH dajoRH removed the WIP-testing label Feb 5, 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.

LGTM, one small comment 👍

@cached_property
def queued_datetime_in_title(self):
return parsetime.from_american_with_utc(self.queued_datetime).to_saved_report_title_format()
delattr(self.appliance, "rest_api")
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps put a small comment about why you need to clear the cache.

@john-dupuy john-dupuy changed the title [RFR] Automate: test_reports_timezone [1LP][RFR] Automate: test_reports_timezone Feb 6, 2020
@digitronik digitronik self-assigned this Feb 7, 2020
Copy link
Contributor

@digitronik digitronik 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 with a first look; I suspect about string if area not found. I will continue review

return cls._parse(cls.saved_report_title_format, time_string)
"""
return cls._parse(
f"{cls.saved_report_title_format} {get_time_difference(area)}", time_string
Copy link
Contributor

Choose a reason for hiding this comment

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

If area not found then it will formate "something None(area)". I think better to avoid this.

@dajoRH dajoRH changed the title [1LP][RFR] Automate: test_reports_timezone [1LP][WIP] Automate: test_reports_timezone Feb 11, 2020
Small changes

Requested changes
@valaparthvi valaparthvi changed the title [1LP][WIP] Automate: test_reports_timezone [1LP][RFR] Automate: test_reports_timezone Feb 11, 2020
@dajoRH
Copy link
Contributor

dajoRH commented Feb 11, 2020

I detected some fixture changes in commit 011b2c8

The local fixture timezone is used in the following files:

  • cfme/tests/intelligence/reports/test_reports.py
    • test_reports_timezone

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

@dajoRH dajoRH removed the WIP label Feb 11, 2020
@digitronik digitronik merged commit 680314c into ManageIQ:master Feb 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement lint-ok test-automation To be applied on PR's which are automating existing manual cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants