Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_post_requests_2 (storageadmin.tests.test_samba.SambaTests) ... ERROR #2327

Closed
phillxnet opened this issue Nov 18, 2021 · 5 comments · Fixed by #2338
Closed

test_post_requests_2 (storageadmin.tests.test_samba.SambaTests) ... ERROR #2327

phillxnet opened this issue Nov 18, 2021 · 5 comments · Fixed by #2338

Comments

@phillxnet
Copy link
Member

phillxnet commented Nov 18, 2021

On a recent source build followed by a full test run on fully updated Leap 15.2 & 15.3 instances (JeOS derived) the following was observed:

======================================================================
ERROR: test_post_requests_2 (storageadmin.tests.test_samba.SambaTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor-dev/eggs/mock-1.0.1-py2.7.egg/mock.py", line 1201, in patched
    return func(*args, **keywargs)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/tests/test_samba.py", line 436, in test_post_requests_2
    response = self.client.post(self.BASE_URL, data=data)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/test.py", line 168, in post
    path, data=data, format=format, content_type=content_type, **extra)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/test.py", line 90, in post
    return self.generic('POST', path, data, content_type, **extra)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/compat.py", line 222, in generic
    return self.request(**r)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/test.py", line 157, in request
    return super(APIClient, self).request(**kwargs)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/test.py", line 109, in request
    request = super(APIRequestFactory, self).request(**kwargs)
  File "/opt/rockstor-dev/eggs/Django-1.8.16-py2.7.egg/django/test/client.py", line 466, in request
    six.reraise(*exc_info)
  File "/opt/rockstor-dev/eggs/Django-1.8.16-py2.7.egg/django/core/handlers/base.py", line 132, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/opt/rockstor-dev/eggs/Django-1.8.16-py2.7.egg/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/opt/rockstor-dev/eggs/Django-1.8.16-py2.7.egg/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/views.py", line 452, in dispatch
    response = self.handle_exception(exc)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/views.py", line 449, in dispatch
    response = handler(request, *args, **kwargs)
  File "/opt/rockstor-dev/eggs/Django-1.8.16-py2.7.egg/django/utils/decorators.py", line 145, in inner
    return func(*args, **kwargs)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/views/samba.py", line 157, in post
    return Response(SambaShareSerializer(smb_share).data)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/serializers.py", line 466, in data
    ret = super(Serializer, self).data
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/serializers.py", line 213, in data
    self._data = self.to_representation(self.instance)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/serializers.py", line 435, in to_representation
    ret[field.field_name] = field.to_representation(attribute)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/serializers.py", line 568, in to_representation
    self.child.to_representation(item) for item in iterable
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/serializers.py", line 426, in to_representation
    attribute = field.get_attribute(instance)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/fields.py", line 299, in get_attribute
    return get_attribute(instance, self.source_attrs)
  File "/opt/rockstor-dev/eggs/djangorestframework-3.1.1-py2.7.egg/rest_framework/fields.py", line 70, in get_attribute
    instance = getattr(instance, attr)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/models/user.py", line 59, in groupname
    return ifp_get_groupname(self.gid)
  File "/opt/rockstor-dev/src/rockstor/system/users.py", line 277, in ifp_get_groupname
    "org.freedesktop.sssd.infopipe", "/org/freedesktop/sssd/infopipe/Groups"
  File "/usr/lib/python2.7/site-packages/dbus/bus.py", line 243, in get_object
    follow_name_owner_changes=follow_name_owner_changes)
  File "/usr/lib/python2.7/site-packages/dbus/proxies.py", line 250, in __init__
    self._named_service = conn.activate_name_owner(bus_name)
  File "/usr/lib/python2.7/site-packages/dbus/bus.py", line 182, in activate_name_owner
    self.start_service_by_name(bus_name)
  File "/usr/lib/python2.7/site-packages/dbus/bus.py", line 280, in start_service_by_name
    'su', (bus_name, flags)))
  File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 653, in call_blocking
    message, timeout)
DBusException: org.freedesktop.DBus.Error.Spawn.ChildExited: Launch helper exited with unknown return code 1

----------------------------------------------------------------------
Ran 216 tests in 18.375s

FAILED (errors=1)

Noting here for reference as this was observed while tending to another issue.

@FroggyFlox
Copy link
Member

Thanks for finally putting that one "in writing". I remember having a try at it a few months back (trying to correctly mock the right call, for instance), but was hitting a snag as it seemed like the fixture was not taking, for instance...

Anyway, thanks a lot for making the effort to open this issue... Hopefully it'll remind me to look into that again.

@phillxnet
Copy link
Member Author

@FroggyFlox Yes I thought we had spoken about this and it completely slipped out of my todo list some how. Apologies for the delay and dropping the ball on this one. As I remember this is currently a cosmetic failure introduced by this test falling behind our main code; and not an indication of core function failure.

@FroggyFlox
Copy link
Member

The error above can be summarized as follows:

Traceback (most recent call last):
  File "/opt/rockstor-dev/eggs/mock-1.0.1-py2.7.egg/mock.py", line 1201, in patched
    return func(*args, **keywargs)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/tests/test_samba.py", line 436, in test_post_requests_2
    response = self.client.post(self.BASE_URL, data=data)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/views/samba.py", line 157, in post
    return Response(SambaShareSerializer(smb_share).data)
  File "/opt/rockstor-dev/src/rockstor/storageadmin/models/user.py", line 59, in groupname
    return ifp_get_groupname(self.gid)
  File "/opt/rockstor-dev/src/rockstor/system/users.py", line 277, in ifp_get_groupname
    "org.freedesktop.sssd.infopipe", "/org/freedesktop/sssd/infopipe/Groups"
DBusException: org.freedesktop.DBus.Error.Spawn.ChildExited: Launch helper exited with unknown return code 1

We can thus see that we fail at the ifp_get_groupname() function, which we thus seem to mock. The call is made in the storageadmin.models.user.groupname method:

@property
def groupname(self, *args, **kwargs):
if self.group is not None:
return self.group.groupname
if self.gid is not None:
try:
groupname = grp.getgrgid(self.gid).gr_name
charset = chardet.detect(groupname)
groupname = groupname.decode(charset["encoding"])
return groupname
except Exception:
# Failed to fetch user using grp, so let's try with infofipe
return ifp_get_groupname(self.gid)
return None

What's interesting is that the error above seems to indicate that the first try statement trying to fetch the groupname using grp fails, and thus reverts to using ifp_get_groupname() which fails as expected because it is not mocked. Now the curious thing is that we do mock grp.getgrgid in our test, which may indicate that we fails in the other two functions of that try block (chardet, charset bits)?

We could alternatively mock ifp_get_groupname() instead as follows:

    @mock.patch("storageadmin.models.user.ifp_get_groupname")
    # @mock.patch("storageadmin.models.user.grp.getgrgid")
    @mock.patch("storageadmin.views.samba.ShareMixin._validate_share")
    @mock.patch("storageadmin.views.samba.User")
    def test_post_requests_2(self, mock_user, mock_validate_share, mock_ifp_get_groupname):
        """
         . Create a samba export for the share that has already been exported
        """
        # # Nullify exception of getgrgid via:
        # mock_getgrgid.side_effects = None

        # Python2
        # import grp
        # >>> groupname = grp.getgrgid("470")
        # >>> print(groupname)
        # grp.struct_group(gr_name='ntp', gr_passwd='x', gr_gid=470, gr_mem=[])
        # return "testgroup" for all gr_name calls:
        # mock_getgrgid.gr_name.return_value = "testgroup"

        # Nullify exception of ifp_get_groupname via:
        mock_ifp_get_groupname.side_effects = None
        mock_ifp_get_groupname.return_value = "testgroup"

        mock_validate_share.return_value = self.temp_share_smb

... making test_post_requests_2() pass:

# ./bin/test -v 2 -p test_samba*
(...)
test_create_samba_share (storageadmin.tests.test_samba.SambaTests) ... ok
test_create_samba_share_existing_export (storageadmin.tests.test_samba.SambaTests) ... ok
test_create_samba_share_incorrect_share (storageadmin.tests.test_samba.SambaTests) ... ok
test_delete_requests_1 (storageadmin.tests.test_samba.SambaTests) ... ok
test_delete_requests_2 (storageadmin.tests.test_samba.SambaTests) ... ok
test_get_non_existent (storageadmin.tests.test_samba.SambaTests) ... ok
test_post_requests_1 (storageadmin.tests.test_samba.SambaTests) ... ok
test_post_requests_2 (storageadmin.tests.test_samba.SambaTests) ... ok
test_post_requests_no_admin (storageadmin.tests.test_samba.SambaTests) ... ok
test_put_requests_1 (storageadmin.tests.test_samba.SambaTests) ... ok
test_put_requests_2 (storageadmin.tests.test_samba.SambaTests) ... ok
test_validate_input (storageadmin.tests.test_samba.SambaTests) ... ok
test_validate_input_error (storageadmin.tests.test_samba.SambaTests) ... ok

----------------------------------------------------------------------
Ran 13 tests in 0.660s

OK

@phillxnet , mocking ifp_get_groupname() seems to work in this case so we could go with that approach; now the one things that troubles me is that we don't seem to have a functioning mock of grp.getgrid anymore, unless we need to also mock the charset/chardet bits if we could that route instead of switching to mocking ifp_get_groupname()... I'll try to see if I can understand that better but in the meantime, do you see a problem with switching to mocking ifp_get_groupname() here?

@phillxnet
Copy link
Member Author

@FroggyFlox Thanks for looking into this a little more.
Re:

@phillxnet , mocking ifp_get_groupname() seems to work in this case so we could go with that approach;
... but in the meantime, do you see a problem with switching to mocking ifp_get_groupname() here?

I think, given there is always more we can, it would be good to have this test back in order: going at least something. We can always return to refine a little more, bit by bit, as we go. And it will likely server us better in some working state than in none. So I say we go with at least getting it doing something. I'd have to take a closer look to say more, just popping in with a quick comment this time around.

FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Jan 2, 2022
@FroggyFlox
Copy link
Member

Thanks for the prompt feedback, @phillxnet!

I tried a bit more to work out why our previous mock of grp.getgrid() was not sufficient anymore but couldn't figure it out, unfortunately... I pushed a branch that mocks ifp_get_groupname() instead and all tests pass:

----------------------------------------------------------------------
Ran 216 tests in 27.902s

OK

I'll get to submitting a PR soon so that we can properly test in Buildbot.

Thanks!

phillxnet added a commit that referenced this issue Jan 4, 2022
Mock ifp_get_groupname() instead of grp.getgrid() #2327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants