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

[1LP][RFR] New Test: test_crud_service_template_with_picture #9774

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

valaparthvi
Copy link
Contributor

Purpose or Intent

  • Adding tests

    1. Add new customer scenario test: test_crud_service_template_with_picture
  • Enhancement

    1. Add rest_api_entity property to
      1. Dialog entity
      2. Catalog entity

PRT Run

{{ pytest: cfme/tests/services/test_rest_services.py -k "test_crud_service_template_with_picture" --long-running -vvvv }}

@valaparthvi valaparthvi changed the title [WIPTEST] New Test: test_crud_service_template_with_picture [RFR] New Test: test_crud_service_template_with_picture Dec 17, 2019
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 :)

2. Note the picture md5 by querying `picture` attribute of the service template.
3. Edit the service template via REST for a different picture.
4. Note the picture md5 by querying `picture` attribute of the service template.
3. Compare both the md5 from testStep 2 and 4.
Copy link
Member

Choose a reason for hiding this comment

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

It should be 5. Compare both the md5 from testStep 2 and 4. Right?

@ganeshhubale ganeshhubale changed the title [RFR] New Test: test_crud_service_template_with_picture [WIPTEST] New Test: test_crud_service_template_with_picture Dec 17, 2019
@valaparthvi valaparthvi changed the title [WIPTEST] New Test: test_crud_service_template_with_picture [RFR] New Test: test_crud_service_template_with_picture Dec 17, 2019
@dajoRH dajoRH added the lint-ok label Dec 17, 2019
@ganeshhubale ganeshhubale changed the title [RFR] New Test: test_crud_service_template_with_picture [1LP][RFR] New Test: test_crud_service_template_with_picture Dec 18, 2019
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 👍
some comments please have a glance.


assert picture_1_md5 != picture_2_md5

service_template.action.delete()
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: why we need this deletion? I think it should be taken care by finalizer.

Copy link
Contributor Author

@valaparthvi valaparthvi Dec 19, 2019

Choose a reason for hiding this comment

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

I could let finalizer do the deletion, but then I thought, since I was already creating and editing the template, I might as well delete it to justify the crud in the test name 😬

Copy link
Contributor

Choose a reason for hiding this comment

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

I think best practice is limit test as per test behavior. If deletion will fails means teardown of test will fail not actual steps. I think better to have separate test for crud operation test.

},
"service_type": "atomic",
"display": True,
"description": fauxfactory.gen_alpha(start="Description "),
Copy link
Contributor

Choose a reason for hiding this comment

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

Required: fauxfactory default character limit is 10

@digitronik digitronik changed the title [1LP][RFR] New Test: test_crud_service_template_with_picture [1LP][WIPTEST] New Test: test_crud_service_template_with_picture Dec 19, 2019
@valaparthvi valaparthvi changed the title [1LP][WIPTEST] New Test: test_crud_service_template_with_picture [1LP][RFR] New Test: test_crud_service_template_with_picture Dec 19, 2019
@digitronik digitronik merged commit 80cb63f into ManageIQ:master Dec 19, 2019
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