Inadvertent host sensitivity re existing user named 'testuser' in UserTests

[phillxnet]

When running our tests on a system with an existing user named testuser, we see an unintended host sensitivity to this user existing:

lbuildvm:/opt/rockstor/src/rockstor # useradd testuser
lbuildvm:/opt/rockstor/src/rockstor # poetry run django-admin test -v 2

======================================================================
FAIL: test_post_requests (rockstor.storageadmin.tests.test_user.UserTests.test_post_requests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/storageadmin/tests/test_user.py", line 195, in test_post_requests
    self.assertEqual(
AssertionError: 200 != 500 : {'id': 3, 'groupname': 'users', 'managed_user': True, 'has_pincard': False, 'pincard_allowed': 'no', 'username': 'newUser', 'uid': 3, 'gid': 5, 'public_key': None, 'shell': '/bin/bash', 'homedir': '/home/newUser', 'email': None, 'admin': True, 'user': 5, 'group': 1, 'smb_shares': []}

----------------------------------------------------------------------
Ran 279 tests in 30.563s

FAILED (failures=1)

With the removal of this user thus:

userdel testuser
no crontab for testuser

Resulting in all tests then passing again:

poetry run django-admin test -v 2

----------------------------------------------------------------------
Ran 279 tests in 31.325s

OK

[Hooverdan96]

Do you want to programmatically resolve this? Or is this something that should be mentioned in the contribution documentation only?

[FroggyFlox]

I would favor the former, personally. Our tests should be able to pass whatever the host system's current state so it means we're missing a mock somewhere, most likely.

[Hooverdan96]

In that case, is it better to just use a very unique user name, or should it be a two-step process, whereas there is an attempt to delete a user by the name of "testuser" before creating one (i.e. that test case)? And maybe a third step where that created test user is being destroyed again before the test cycle is over? So something like running in sequence the existing test cases?

1) something like the method test_delete_existing_user(self) - since this step is only for cleanup in case testuser already exists, it would not be considered a failure 2) test_post_requests(self) 3) test_delete_existing_user(self)

Or could the testuser be changed to one that e.g. is a combination of testuser appended with a randomly generated number (That has probably some implications that would require other test cases to be adjusted that use testuser currently?

[FroggyFlox]

Ideally we shouldn't need to do any of that and the test in itself should be isolated enough to be robust against this sort of things.

Now, it seems that the failing test show was the following symptom: the POST request to create a user with an already used uid does not fail while it should.

The test is already using fixtures properly, but I noticed that these uid/good in the fixture are int... maybe we now need to recreate that fixture or simply convert these to str... if that's the case, there may be even more implications but we'll need to see that fixture "issue" first.

It could also be the consequence of the previous POST calls in that same test but it needs more exploration here.

[phillxnet]

@FroggyFlox I've just notice you already reported this exact same issue; so closing this as a duplicate thus:

Duplicate of #2647

See: "Insufficient mocking in test_user.py" #2647