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

[1LP][RFR] Create test_import_tag and implement fixtures and functions it needs #10162

Merged
jawatts merged 11 commits into from
Jun 19, 2020
Merged

[1LP][RFR] Create test_import_tag and implement fixtures and functions it needs #10162

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
87f5a36
Begin work on test_import_tag and create navigation and view for impo…
prichard77 Jun 3, 2020
70716bd
Implement upload_file as FileInput
prichard77 Jun 4, 2020
a527410
Add creation of csv file
prichard77 Jun 4, 2020
c29f23f
Add usage of tag fixture and fix csv file format
prichard77 Jun 5, 2020
54bb6fd
Add apply button and usage and use create_vm
prichard77 Jun 5, 2020
43650ff
Move csv file creation into a fixture
prichard77 Jun 5, 2020
3e390b1
Use Tempfile in csv_tag_file fixture
prichard77 Jun 5, 2020
bd33e8f
Reorder imports
prichard77 Jun 5, 2020
6e9f5f3
Add to test_import_tag doc block
prichard77 Jun 5, 2020
a3bbb5b
Add check that tag is added to VM
prichard77 Jun 5, 2020
0438283
Move vm tag validation assertion out of method and into test case
prichard77 Jun 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions cfme/configure/configuration/region_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from widgetastic.exceptions import RowNotFound
from widgetastic.exceptions import UnexpectedAlertPresentException
from widgetastic.widget import Checkbox
from widgetastic.widget import FileInput
from widgetastic.widget import Select
from widgetastic.widget import Text
from widgetastic_patternfly import BootstrapSelect
Expand Down Expand Up @@ -57,6 +58,17 @@ def is_displayed(self):
)


class CompanyTagsImportView(RegionView):
"""Import Company Tags list view"""
upload_file = FileInput(id="upload_file")
upload_button = Button(id='upload_tags')
apply_button = Button("Apply")

@property
def is_displayed(self):
return (self.tags.is_active() and self.import_tags.is_active())


@attr.s(eq=False)
class Tag(Pretty, BaseEntity, Updateable):
""" Class represents a category in CFME UI
Expand Down Expand Up @@ -266,6 +278,20 @@ def delete(self, cancel=False):
assert view.is_displayed
view.flash.assert_no_error()

def import_tag_from_file(self, file_name):
"""Import Tag from CSV formatted file

Args:
file_name: Name of .csv file containing tag data.
By default file must be at cfme/tests/configure/
Copy link
Member

@mshriver mshriver Jun 5, 2020

Choose a reason for hiding this comment

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

Why must the file be at this location? tempfile should take care of the file location, and it shouldn't matter where the file is for the import in the UI.

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

Removed the comment. his is where I was seeing the framework put the file. I guess it handles this location so we don't need to care about it.

"""
view = navigate_to(self.parent, 'ImportTags')
view.upload_file.fill(file_name)
view.upload_button.click()
view.flash.assert_no_error()
view.apply_button.click()
view.flash.assert_no_error()

@property
def exists(self):
"""Check if category exists"""
Expand Down Expand Up @@ -342,15 +368,22 @@ def step(self, *args, **kwargs):
self.prerequisite_view.add_button.click()


@navigator.register(CategoriesCollection, 'ImportTags')
class CategoryImportTags(CFMENavigateStep):
VIEW = CompanyTagsImportView
prerequisite = NavigateToAttribute('appliance.server.zone.region', 'Details')

def step(self, *args, **kwargs):
self.prerequisite_view.tags.import_tags.select()


@navigator.register(Category, 'Edit')
class CategoryEdit(CFMENavigateStep):
VIEW = CompanyCategoriesEditView
prerequisite = NavigateToAttribute('parent', 'All')

def step(self, *args, **kwargs):
self.prerequisite_view.table.row(name=self.obj.name).click()


# =======================================MAP TAGS==============================================


Expand Down
25 changes: 25 additions & 0 deletions cfme/tests/configure/test_tag.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import tempfile

import fauxfactory
import pytest

Expand Down Expand Up @@ -124,6 +126,29 @@ def test_map_tagging_crud(appliance, category, soft_assert):
.format(map_tag_entity.label))


@pytest.fixture
def csv_tag_file(create_vm, category, tag):
temp_file = tempfile.NamedTemporaryFile(suffix=".csv")
Copy link
Member

@mshriver mshriver Jun 5, 2020

Choose a reason for hiding this comment

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

So, you can use tempfiles's context manager here to make this even simpler.

with tempfile.NamedTemporaryFile(suffix='.csv') as temp_file:
    tempfile.write(csv_data)
    yield temp_file.name

Note this is yielding the file name, not the file object, which is what the test actually needs anyway.

After the yield, the context manager exists and the file is deleted

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

Updated as suggested. I was on the fence about returning the object or just name. I relaize I just use the name, but was thinking I might want to access the object if I used this fixture for another test. Using it in another test seems quite unlikely now.

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

getting error trying to use tempfile: TypeError: expected str, bytes or os.PathLike object, not _TemporaryFileWrapper . Investigating.

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

I was unable to get the shortened version to work. Tried a few things but was getting errors or creating empty files. Moving on to the asserts for the vm tag.

csv_data = f'name,category,entry\n{create_vm.name},{category.display_name},{tag.display_name}'
Copy link
Member

@mshriver mshriver Jun 5, 2020

Choose a reason for hiding this comment

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

Shouldn't you be using a random alphanumeric for the tag, instead of trying to import a tag that matches an existing one?

The tag fixture that you're using here is a random tag that has already been created on the category.

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

yes. The import is to add the existing tag value to a VM. If the tag or category or VM do not exist, the upload fails. the terms here are confusing between a category, entry, tag etc. . I believe i have the bahavior ad it is in the BZ.

Copy link
Member

@mshriver mshriver Jun 5, 2020

Choose a reason for hiding this comment

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

Can you add to the docblock for the test case the description that you're importing tag mappings to a VM, and not just tag definitions.

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

Done

# create CSV file
with open(temp_file.name, "w") as file:
file.write(csv_data)
yield file


@test_requirements.tag
def test_import_tag(appliance, create_vm, category, tag, csv_tag_file):
"""Test importing tag via file
Polarion:
assignee: prichard
initialEstimate: 1/4h
casecomponent: Tagging
Bugzilla:
1792185
"""
category.import_tag_from_file(csv_tag_file.name)
Copy link
Member

@mshriver mshriver Jun 5, 2020

Choose a reason for hiding this comment

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

I think some additional assertions are necessary here - you have the tag and category display names, you should be able to instantiate a Tag and assert that it exists.

Look at something like:

assert category.tags.instantiate(display_name=tag.display_name).exists

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

If the tag does not exist the import will not work. The BZ issue was that an error gets displayed after "Upload" is clicked but before the apply. We are covering the BZ here. I guess I could add an assert that the tag exists on the VM?

Copy link
Member

@mshriver mshriver Jun 5, 2020

Choose a reason for hiding this comment

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

gotcha, that makes more sense - please update the docblock with more detailed description of what's being tested.

Also correct about the additional assertion of the tag on the VM. You should be able to call tag in create_vm.get_tags, as the tag has an __eq__ implementation.

Copy link
Contributor Author

@prichard77 prichard77 Jun 5, 2020

Choose a reason for hiding this comment

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

I put the assert against the vm tags in the category.import_tag_from_file() method.
I could have put it in the test case itself, but chose the method.
If I put it in the testcase I wouldn't need to pass down the extra 2 args to import_tag_from_file and could leave more flexibility in other tests to check results for an already existing tag (just comparing the display name strings) without having to create the tag object.
But how much more tagging tests are we going to write? And if we do write more, couldn't I just move the assert out then?

Copy link
Contributor Author

@prichard77 prichard77 Jun 8, 2020

Choose a reason for hiding this comment

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

I moved the validation of the tag on the VM out into the testcase.



@test_requirements.tag
def test_updated_tag_name_on_vm(provider, tag, request):
"""
Expand Down