Rewrite Entity.__init__, add sync plan tests
#1969
Conversation
| ).create_json()['id'] | ||
| cls.prod2_id = entities.Product( | ||
| organization=cls.org_id | ||
| ).create_json()['id'] |
There was a problem hiding this comment.
I've dropped this because of BZ 1199150. Re-using these products for multiple tests causes unnecessary test failures. test_repeatedly_add_remove targets this issue.
|
ACK pending clarification of comments. |
|
This pull request has been completely updated to take care of this issue. I tweaked But when an entity is read back, this yields an The tweak I made will make that fix easier to implement in the future. |
| syncplan.add_products(product_ids) | ||
| self.assertEqual(len(syncplan.read_json()['products']), 2) | ||
| syncplan.remove_products([product_ids[0]]) | ||
| self.assertEqual(len(syncplan.read_json()['products']), 1) |
There was a problem hiding this comment.
Would it be too much to ask that you assert that the product 'left' is the one you did not delete? Just to be safe?
There was a problem hiding this comment.
Sure. I'll do this.
Ichimonji10
changed the title
Ichimonji10
changed the title
Entity.__init__, add sync plan tests
Ichimonji10
changed the title
Entity.__init__, add sync plan testsEntity.__init__, add sync plan tests
|
ACK |
|
If anyone is confused by the changes in this pull request (and I don't blame you!), feel free to ask me for clarification. I'll walk you through all the changes I've made. |
| @@ -1321,7 +1330,8 @@ def create_missing(self, auth=None): | |||
| verify=False, | |||
| data={ | |||
| u'name': u'Library', | |||
| u'organization_id': self.organization, | |||
| # pylint:disable=no-member | |||
| u'organization_id': self.organization.id, | |||
|
ACK, pending comments. |
Method `Entity.__init__` is buggy. The bug lies in the fact that entity
creation behaves differently from entity reading. For example:
>>> SomeEntity(related_entity=1).related_entity == 1
True
>>> SomeEntity(id=N).read().related_entity == 1
False
This is problematic, because it is hard to know what type of object a foreign
key field is. Will a foreign key field be an integer? An `Entity` object? That's
hard to determine. Fix this issue by making `Entity.__init__` behave like
`EntityReadMixin.read`:
>>> isinstance(SomeEntity(related_entity=1).related_entity, RelatedEntity)
True
>>> isinstance(SomeEntity(id=N).read().related_entity, RelatedEntity)
True
Explain the new behaviour in `Entity`'s docstring.
Take advantage of this new behaviour and rewrite both the
`OperatingSystemEntity` and `SyncPlan` entity classes. Add a test for
`SyncPlan.read` to module `tests.foreman.api.test_multiple_paths`. Update a test
for `OperatingSystemParameter` in module
`tests.foreman.api.test_operatingsystem`.
Test that it is possible to add products to and remove products from a sync plan. These tests target the following BZs: * [1132914](https://bugzilla.redhat.com/show_bug.cgi?id=1132914): "Duplicate route sync plan" * [1199150](https://bugzilla.redhat.com/show_bug.cgi?id=1199150): "Adding or removing products from a syncplan should return a task" Also, fiddle with decorators. Remove unnecessary `@ddt` decorators from classes and add `@stubbed` decorators to them. The addition of the `@stubbed` decorators prevents unnecessary `setUpClass` methods from executing.
|
@elyezer, I've Rebased. |
|
ACK |
Rewrite `Entity.__init__`, add sync plan tests
|
Karma given |
See individual commit messages for details.
The changes made in this PR are extensive, so I decided that it was appropriate to run the entire test suite. Here's the results:
Twelve of the failures were due to my
robottelo.propertiesfile not containing docker configuration options. After updating myrobottelo.propertiesfiles, I am convinced that they are fine. Several more failures are due to abrt-related permissions not being returned from the server. The remaining failures are due to the server rejecting the bogus manifest submitted during thetest_subscriptiontests. All in all, the test results are completely sane.