Implemented Entity eq method comparing all fields #350

renzon · 2017-01-03T17:48:38Z

close #335

coveralls · 2017-01-03T18:27:58Z

Coverage decreased (-0.1%) to 98.478% when pulling 0edac6c on renzon:335 into ba008a5 on SatelliteQE:master.

rochacbruno

ACK
but misses a little of test coverage in 2 lines/conditions

rochacbruno · 2017-01-04T09:40:22Z

nailgun/entity_mixins.py

@@ -551,17 +552,30 @@ def to_json_dict(self):
        json_dct = {}
        for field_name, field in fields.items():
            if field_name in values:
+                value = values[field_name]
+                if value is None:
+                    json_dct[field_name] = None


coveralls says this line misses test coverage

rochacbruno · 2017-01-04T09:40:47Z

nailgun/entity_mixins.py

+        :return: boolean indicating if entities are equal or not
+        """
+        if other is None:
+            return False


This line also missing test coverage

renzon · 2017-01-04T11:05:30Z

@rochacbruno coverage improved.

renzon · 2017-01-04T11:06:14Z

For the one who is going to merge: please use "Squash and Merge" Once I fixed code on different commit.

close SatelliteQE#335

coveralls · 2017-01-04T11:10:29Z

Coverage increased (+0.005%) to 98.583% when pulling 7d82a01 on renzon:335 into ba008a5 on SatelliteQE:master.

elyezer

ACK, pending comments.

elyezer · 2017-01-06T17:55:07Z

nailgun/entity_mixins.py

                    ]
                else:
-                    json_dct[field_name] = to_json_serializable(
-                        values[field_name])
+                    json_dct[field_name] = to_json_serializable(value)


I did the following test:

In [1]: from nailgun.entity_mixins import to_json_serializable In [2]: to_json_serializable(None) In [3]: print(to_json_serializable(None)) None

There is no need to treat the value is None since the else clause will already take care of it.

At first I was doing it. But discovered a case during test on robottelo where an OneToOne is None and. 'json_dct[field_name] = value.to_json_dict()' lead to error because to_json_dict() is been called on a None value.

Finnaly found the case I mentioned above:

hostgroup = entities.HostGroup( location=[self.loc], organization=[self.org], ).create() hostgroup.to_json_dict()

Stack trace:

def to_json_dict(self): """Create a dct with Entity properties for json encoding. It can be overridden by subclasses for each standard serialization doesn't work. By default it call _to_json_dict on OneToOne fields and build a list calling the same method on each object on OneToMany fields. :return: dct """ fields, values = self.get_fields(), self.get_values() json_dct = {} for field_name, field in fields.items(): if field_name in values: if isinstance(field, OneToOneField): > json_dct[field_name] = values[field_name].to_json_dict() E AttributeError: 'NoneType' object has no attribute 'to_json_dict'

I see, do you mind adding a comment on that if clause? It may happen that someone else realize that the json_dct[field_name] = to_json_serializable(value) works for None and is not aware of that specific case.

@elyezer: done!

elyezer · 2017-01-06T17:58:46Z

tests/test_entity_mixins.py

+
+        """
+        alice = SampleEntity(self.cfg, id=1, name='Alice')
+        self.assertNotEquals(alice, None)


Use assertNotEqual (without the s at the end) since assertNotEquals is deprecated and will produce a warning:

In [7]: t.assertNotEquals Out[7]: <bound method TestCase._deprecate.<locals>.deprecated_func of <__main__.Test testMethod=runTest>> In [9]: t.assertNotEquals?? Signature: t.assertNotEquals(*args, **kwargs) Source: def deprecated_func(*args, **kwargs): warnings.warn( 'Please use {0} instead.'.format(original_func.__name__), DeprecationWarning, 2) return original_func(*args, **kwargs) File: /usr/lib64/python3.5/unittest/case.py Type: method

Same applies for assertEquals which should be assertEqual

coveralls · 2017-01-09T12:40:39Z

Coverage increased (+0.005%) to 98.583% when pulling 331ce50 on renzon:335 into ba008a5 on SatelliteQE:master.

elyezer

I will approve the PR, thanks for the clarification. The last comment is just to make sure we know why the new if block is needed in the future.

elyezer · 2017-01-09T15:49:16Z

nailgun/entity_mixins.py

                    ]
                else:
-                    json_dct[field_name] = to_json_serializable(
-                        values[field_name])
+                    json_dct[field_name] = to_json_serializable(value)


I see, do you mind adding a comment on that if clause? It may happen that someone else realize that the json_dct[field_name] = to_json_serializable(value) works for None and is not aware of that specific case.

coveralls · 2017-01-09T16:35:07Z

Coverage decreased (-0.05%) to 98.531% when pulling 3f667b7 on renzon:335 into ba008a5 on SatelliteQE:master.

coveralls · 2017-01-09T16:36:41Z

Coverage increased (+0.005%) to 98.583% when pulling 3f667b7 on renzon:335 into ba008a5 on SatelliteQE:master.

coveralls · 2017-01-09T16:36:41Z

Coverage increased (+0.005%) to 98.583% when pulling 3f667b7 on renzon:335 into ba008a5 on SatelliteQE:master.

rochacbruno · 2017-01-09T21:52:06Z

tests/test_entity_mixins.py

+
+        """
+        alice = SampleEntity(self.cfg, id=1, name='Alice')
+        self.assertFalse(alice == None)


Comparison to None should be alice is None instead of ==

Good catch, better to use self.assertIsNotNone(alice)

The point is that I want eq to be called with None. So I am going to explicitly call it ;)

close SatelliteQE#335

coveralls · 2017-01-10T13:45:18Z

Coverage increased (+0.005%) to 98.583% when pulling 93e0097 on renzon:335 into ba008a5 on SatelliteQE:master.

renzon · 2017-01-10T13:46:51Z

@rochacbruno take a look on the change

(cherry picked from commit ecf91be)

close #335 (cherry picked from commit ecf91be)

close SatelliteQE#335 (cherry picked from commit ecf91be)

(cherry picked from commit ecf91be)

(cherry picked from commit ecf91be) (cherry picked from commit f231e8c)

Shortlog of commits since last release: Andrii Balakhtar (4): Fix read() of hostgroup without content_source, lce or cv for 6.1 Increase timeout for manifest delete/refresh (SatelliteQE#359) Provide default name length & type for ComputeResource entities (SatelliteQE#369) Implemented Interface entity with all its fields and basic operations (SatelliteQE#373) Djebran Lezzoum (1): Merge pull request SatelliteQE#380 from svtkachenko/ct_update_payload Elyézer Rezende (1): Update travis config Oleksandr Shtaier (8): Merge pull request SatelliteQE#341 from sghai/update-test Merge pull request SatelliteQE#342 from renzon/332 Merge pull request SatelliteQE#344 from elyezer/update-travis Merge pull request SatelliteQE#352 from svtkachenko/sv_payload Merge pull request SatelliteQE#345 from abalakh/fix_hg_61_workaround Merge pull request SatelliteQE#366 from svtkachenko/update_template_kind Merge pull request SatelliteQE#361 from svtkachenko/update_sv Merge pull request SatelliteQE#383 from svtkachenko/63_image Renzo Nuccitelli (3): Added json method to Entity (SatelliteQE#326) Merge pull request SatelliteQE#347 from svtkachenko/add_image Implemented Entity __eq__ method comparing all fields (SatelliteQE#350) Stanislav Tkachenko (10): Add content related helpers for Repository entity. (SatelliteQE#327) Add Create and Delete mixins to Smart Proxy (SatelliteQE#343) Add mixins to Image Override create_payload and upload_payload for SmartVariable. Update Host.puppet_class field name. (SatelliteQE#346) Removed overriden read method for Smart Variable Update TemplateKind entity Add ProvisioningTemplate. (SatelliteQE#365) Fix update_payload method for ConfigTemplate/ProvisioningTemplate [6.2.z] Update Image and Host entities (SatelliteQE#356) oshtaier (2): Add UpdateMixin for CV Filter Rules entity Add host count field to CV entity renzon (5): Fixed ConfigTemplate create and update with TemplateCombination ListFiled handled on _payload api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination sghai (5): Updated discovery_rule entity with org & location (SatelliteQE#320) Added a new field root_pass to hostgroup entity (SatelliteQE#336) Added unit test for root_pass from hostgroup entity Merge pull request SatelliteQE#338 from oshtaier/cvf_update_mixin Merge pull request SatelliteQE#340 from oshtaier/cv_field

(cherry picked from commit ecf91be) (cherry picked from commit f231e8c)

* Implemented Entity __eq__ method comparing all fields close SatelliteQE#335 * Improved Coverage close SatelliteQE#335 * Improved Coverage close SatelliteQE#335

Shortlog of commits since last release: Andrii Balakhtar (4): Fix read() of hostgroup without content_source, lce or cv for 6.1 Increase timeout for manifest delete/refresh (SatelliteQE#359) Provide default name length & type for ComputeResource entities (SatelliteQE#369) Implemented Interface entity with all its fields and basic operations (SatelliteQE#373) Djebran Lezzoum (1): Merge pull request SatelliteQE#380 from svtkachenko/ct_update_payload Elyézer Rezende (1): Update travis config Oleksandr Shtaier (8): Merge pull request SatelliteQE#341 from sghai/update-test Merge pull request SatelliteQE#342 from renzon/332 Merge pull request SatelliteQE#344 from elyezer/update-travis Merge pull request SatelliteQE#352 from svtkachenko/sv_payload Merge pull request SatelliteQE#345 from abalakh/fix_hg_61_workaround Merge pull request SatelliteQE#366 from svtkachenko/update_template_kind Merge pull request SatelliteQE#361 from svtkachenko/update_sv Merge pull request SatelliteQE#383 from svtkachenko/63_image Renzo Nuccitelli (3): Added json method to Entity (SatelliteQE#326) Merge pull request SatelliteQE#347 from svtkachenko/add_image Implemented Entity __eq__ method comparing all fields (SatelliteQE#350) Stanislav Tkachenko (10): Add content related helpers for Repository entity. (SatelliteQE#327) Add Create and Delete mixins to Smart Proxy (SatelliteQE#343) Add mixins to Image Override create_payload and upload_payload for SmartVariable. Update Host.puppet_class field name. (SatelliteQE#346) Removed overriden read method for Smart Variable Update TemplateKind entity Add ProvisioningTemplate. (SatelliteQE#365) Fix update_payload method for ConfigTemplate/ProvisioningTemplate [6.2.z] Update Image and Host entities (SatelliteQE#356) oshtaier (2): Add UpdateMixin for CV Filter Rules entity Add host count field to CV entity renzon (5): Fixed ConfigTemplate create and update with TemplateCombination ListFiled handled on _payload api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination sghai (5): Updated discovery_rule entity with org & location (SatelliteQE#320) Added a new field root_pass to hostgroup entity (SatelliteQE#336) Added unit test for root_pass from hostgroup entity Merge pull request SatelliteQE#338 from oshtaier/cvf_update_mixin Merge pull request SatelliteQE#340 from oshtaier/cv_field

renzon added the in progress label Jan 3, 2017

renzon force-pushed the 335 branch 3 times, most recently from 2eaa27f to e900700 Compare January 3, 2017 18:18

Implemented Entity __eq__ method comparing all fields

0edac6c

close SatelliteQE#335

renzon force-pushed the 335 branch from e900700 to 0edac6c Compare January 3, 2017 18:26

renzon requested review from rochacbruno and abalakh January 3, 2017 18:29

renzon self-assigned this Jan 3, 2017

renzon added review and removed in progress labels Jan 3, 2017

rochacbruno suggested changes Jan 4, 2017

View reviewed changes

Improved Coverage

7d82a01

close SatelliteQE#335

renzon force-pushed the 335 branch from 247d7e2 to 7d82a01 Compare January 4, 2017 11:08

rochacbruno approved these changes Jan 4, 2017

View reviewed changes

renzon requested review from elyezer and svtkachenko January 4, 2017 13:13

elyezer suggested changes Jan 6, 2017

View reviewed changes

renzon removed request for abalakh and svtkachenko January 9, 2017 12:39

elyezer approved these changes Jan 9, 2017

View reviewed changes

renzon force-pushed the 335 branch 2 times, most recently from cd72f5d to 3f667b7 Compare January 9, 2017 16:34

renzon force-pushed the 335 branch from 3f667b7 to eb1fed2 Compare January 9, 2017 16:38

rochacbruno suggested changes Jan 9, 2017

View reviewed changes

Improved Coverage

93e0097

close SatelliteQE#335

renzon force-pushed the 335 branch from eb1fed2 to 93e0097 Compare January 10, 2017 13:43

rochacbruno approved these changes Jan 10, 2017

View reviewed changes

rochacbruno merged commit ecf91be into SatelliteQE:master Jan 10, 2017

rochacbruno removed the review label Jan 10, 2017

renzon added a commit to renzon/nailgun that referenced this pull request Jan 16, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

3b5f391

(cherry picked from commit ecf91be)

renzon added a commit that referenced this pull request Jan 16, 2017

Implemented Entity __eq__ method comparing all fields (#350)

32ad3c6

close #335 (cherry picked from commit ecf91be)

renzon added a commit to renzon/nailgun that referenced this pull request Jan 16, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

1857993

close SatelliteQE#335 (cherry picked from commit ecf91be)

renzon added a commit to renzon/nailgun that referenced this pull request Jan 16, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

3ab8d82

(cherry picked from commit ecf91be)

renzon added a commit to renzon/nailgun that referenced this pull request Jan 16, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

e607922

(cherry picked from commit ecf91be)

renzon added a commit to renzon/nailgun that referenced this pull request Jan 16, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

f231e8c

(cherry picked from commit ecf91be)

rochacbruno pushed a commit that referenced this pull request Jan 16, 2017

Implemented Entity __eq__ method comparing all fields (#350) (#353)

398e7c5

(cherry picked from commit ecf91be)

renzon added a commit to renzon/nailgun that referenced this pull request Jan 19, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

8dab246

(cherry picked from commit ecf91be) (cherry picked from commit f231e8c)

renzon added a commit to renzon/nailgun that referenced this pull request Jan 19, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

09c3dc4

(cherry picked from commit ecf91be) (cherry picked from commit f231e8c)

renzon added a commit to renzon/nailgun that referenced this pull request Jan 19, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

1e6221c

(cherry picked from commit ecf91be) (cherry picked from commit f231e8c)

rochacbruno pushed a commit that referenced this pull request Jan 23, 2017

Implemented Entity __eq__ method comparing all fields (#350) (#358)

524980a

(cherry picked from commit ecf91be) (cherry picked from commit f231e8c)

renzon pushed a commit to renzon/nailgun that referenced this pull request Mar 9, 2017

Implemented Entity __eq__ method comparing all fields (SatelliteQE#350)

a9bbd1b

(cherry picked from commit ecf91be) (cherry picked from commit f231e8c)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented Entity eq method comparing all fields #350

Implemented Entity eq method comparing all fields #350

renzon commented Jan 3, 2017

coveralls commented Jan 3, 2017 •

edited

rochacbruno left a comment

rochacbruno Jan 4, 2017

rochacbruno Jan 4, 2017

renzon commented Jan 4, 2017

renzon commented Jan 4, 2017

coveralls commented Jan 4, 2017 •

edited

elyezer left a comment

elyezer Jan 6, 2017

renzon Jan 9, 2017

renzon Jan 9, 2017

elyezer Jan 9, 2017

renzon Jan 9, 2017

elyezer Jan 6, 2017

elyezer Jan 6, 2017

coveralls commented Jan 9, 2017 •

edited

elyezer left a comment

elyezer Jan 9, 2017

coveralls commented Jan 9, 2017

coveralls commented Jan 9, 2017

coveralls commented Jan 9, 2017

rochacbruno Jan 9, 2017

elyezer Jan 10, 2017 •

edited

renzon Jan 10, 2017 •

edited

coveralls commented Jan 10, 2017 •

edited

renzon commented Jan 10, 2017

Implemented Entity __eq__ method comparing all fields #350

Implemented Entity __eq__ method comparing all fields #350

Conversation

renzon commented Jan 3, 2017

coveralls commented Jan 3, 2017 • edited

rochacbruno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renzon commented Jan 4, 2017

renzon commented Jan 4, 2017

coveralls commented Jan 4, 2017 • edited

elyezer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 9, 2017 • edited

elyezer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 9, 2017

coveralls commented Jan 9, 2017

coveralls commented Jan 9, 2017

Choose a reason for hiding this comment

elyezer Jan 10, 2017 • edited

Choose a reason for hiding this comment

renzon Jan 10, 2017 • edited

Choose a reason for hiding this comment

coveralls commented Jan 10, 2017 • edited

renzon commented Jan 10, 2017

Implemented Entity eq method comparing all fields #350

Implemented Entity eq method comparing all fields #350

coveralls commented Jan 3, 2017 •

edited

coveralls commented Jan 4, 2017 •

edited

coveralls commented Jan 9, 2017 •

edited

elyezer Jan 10, 2017 •

edited

renzon Jan 10, 2017 •

edited

coveralls commented Jan 10, 2017 •

edited