Refs #23676 - Robottelo tests attr - content_view #7527
Conversation
|
Issues: #23676 |
|
[test katello/webpack] |
| @@ -179,6 +189,13 @@ def test_update_protected | |||
| end | |||
| end | |||
|
|
|||
| test_attributes :pid => '69a2ce8d-19b2-49a3-97db-a1fdebbb16be' | |||
| def test_should_not_update_with_invalid_name | |||
There was a problem hiding this comment.
I'd prefer test_should_not_update_with_empty_name, which is more precise.
| @@ -91,6 +93,13 @@ def test_create_fail_without_organization_id | |||
| assert_response :not_found | |||
| end | |||
|
|
|||
| test_attributes :pid => '261376ca-7d12-41b6-9c36-5f284865243e' | |||
| def test_should_not_create_with_invalid_name | |||
There was a problem hiding this comment.
I'd prefer test_should_not_create_with_empty_name, which is more precise.
| test_attributes :pid => '4a3b616d-53ab-4396-9a50-916d6c42a401' | ||
| def test_should_create_composite | ||
| view = ContentView.new(:name => "new content view", :organization_id => @organization.id) | ||
| [false, true].each do |composite| |
There was a problem hiding this comment.
Why test non-composite here? I'd opt for testing only composite, since we test normal CV creating on L80 test_create_with_name.
There was a problem hiding this comment.
@akofink we need to trigger both property values for this test, we have to be sure that we are able to create with with true false values.
There was a problem hiding this comment.
@akofink this test test cost nothing in terms of time, as it does create a real entity, but informative on the reason of failure (in case it fail)
There was a problem hiding this comment.
this test test cost nothing in terms of time
All the tests have some time cost associated, even if it doesn't hit the DB. Ensuring we don't test the same thing twice isn't pertinent, but it's ideal. Since L80 is testing composite: nil (i.e. default) instead of composite: false, I guess this is fine.
| view = ContentView.create!(:name => "org content view", :organization_id => @organization.id) | ||
| valid_name_list.each do |name| | ||
| view.name = name | ||
| assert view.valid?, "Validation failed for content view update with valid name: '#{name}', length: #{name}" |
There was a problem hiding this comment.
This is tested on L66 with should_not allow_values(*invalid_name_list).for(:name).
There was a problem hiding this comment.
@akofink should_not allow_values this is a negative test, but here we are covering a positive test scenario
There was a problem hiding this comment.
If you want to test valid_name_list names are accepted here, then at least, let's not test them all in other places (i.e. L227).
There was a problem hiding this comment.
@akofink in L227 we are testing description field with variation values using name list
There was a problem hiding this comment.
The name and description validations in the controller have no dependencies - they're both validated individually. You should either be testing the name field validations or the description validations, not both. These are unit tests after all!
| end | ||
|
|
||
| test_attributes :pid => '77883887-800f-412f-91a3-b2f7ed999c70' | ||
| def test_should_not_update_label |
There was a problem hiding this comment.
This is covered on L108 test_bad_label.
There was a problem hiding this comment.
@akofink That test is testing the value of label that is not acceptable, here we are testing that label is immutable and cannot be changed even if the value is valid
ldjebran
force-pushed
the
content_view_test_attributes
branch
from
f09b408
to
8199b62
Compare
|
@akofink updated test names and rebased |
|
@akofink mind re-reviewing? |
Related robottelo issues:
SatelliteQE/robottelo#6125
SatelliteQE/robottelo#5843
Update to PR: #7395
Port robottelo tier1 tests for organization part of robottelo minitest port project: https://github.com/SatelliteQE/robottelo/projects/1