Introduce setup and teardown fixtures

[Shwetha-Acharya]

With this change, multiple test modules triggered from testcases/mount can be run as individual tests complying to pytest standards. Also any number of new tests can be added efficiently on the mount.

Old approach:

• All the different tests are called from test_mount.py:
  https://github.com/samba-in-kubernetes/sit-test-cases/blob/main/testcases/mount/test_mount.py
• Test_mount.py creates a mount, the required tests are called and then test_mount.py cleans the mount.

New approach :

• This PR complies with the proposal to use fixtures in order to remove all the load on one single
  module:
  mount: Add a stress test with multiple clients #25 (comment)
  mount: Add a stress test with multiple clients #25 (comment)

• The tests are no more called from test_mount.py.

• Setting up of mount and tearing down of mount is taken care by setup and teardown fixture
  which is defined in testcases/mount/conftest.py

  More information about fixtures:
  https://docs.pytest.org/en/6.2.x/fixture.html

• With this change, test files remain the same, but the initiator function starts with keyword test
  and the each test module starts with keyword 'test'

  That way the tests are called by pytest.
  Usage of setup and teardown fixture becomes easily possible.
  We no more need to to call the test function explictly.

  This also helps with easy debugging and better understanding of the code, as the tests will be
  failed at individual mount/IO test.

  In this case, pytest clearly points which test among test_mount_io.py, test_mount_dbm.py
  and test_mount_stress.py are failing.
  Can be very useful

  • when more and more tests are added
  • when we want to parse test results in sit-environment project

Fixes: #30

[Shwetha-Acharya]

This PR need few changes to be merged to pass the tests

[anoopcs9]

This PR need few changes to be merged to pass the tests

Please feel free to edit the description to add the keyword depends on #<PR.no> so that its visible in the list of checks.

[Shwetha-Acharya]

This PR need few changes to be merged to pass the tests

Please feel free to edit the description to add the keyword depends on #<PR.no> so that its visible in the list of checks.

Done!

[anoopcs9]

centos-ci/xfs seems to have failed . @Shwetha-Acharya can you check if its related to the proposed change?

[spuiuk]

some minor changes. The rest looks good.

You can also go ahead with re-adding tests for pre-mounted shares after making those changes.

Thanks.

[Shwetha-Acharya]

@spuiuk sit environment is not adding any premounted share value inside test-info created by it:

Refer test-info.yaml on client from build artefacts of client in: https://jenkins-samba.apps.ocp.cloud.ci.centos.org/job/samba_cephfs.vfs-integration-test-cases/125/

private_interfaces:
  - 192.168.122.100
  - 192.168.122.101
  - 192.168.122.102


public_interfaces:
  - 192.168.123.10
  - 192.168.123.11
  - 192.168.123.12


exported_sharenames:
  - share-cephfs-default


test_users:
  -   password: x
      uid: '2001'
      username: test1
  -   password: x
      uid: '2002'
      username: test2


test_backend: cephf

FAILED testcases/smbtorture/test_smbtorture.py::test_smbtorture[share-cephfs-vfs-smb2.create]
SKIPPED [1] testcases/mount/test_mount_io.py:142: got empty parameter set ['test_dir'], function test_check_io_consistency_premounted at /root/sit-test-cases/testcases/mount/test_mount_io.py:141
SKIPPED [1] testcases/mount/test_mount_stress.py:68: got empty parameter set ['test_dir'], function test_dbm_consistency_premounted at /root/sit-test-cases/testcases/mount/test_mount_stress.py:67
SKIPPED [1] testcases/mount/test_mount_dbm.py:110: got empty parameter set ['test_dir'], function test_dbm_consistency_premounted at /root/sit-test-cases/testcases/mount/test_mount_dbm.py:109

Also the same code pass, when run locally by changing test-info.yml.example to test-info.yml (This is because the premounted share value is now present)

testcases/mount/test_mount_io.py::test_check_io_consistency_premounted[test_dir0] 

PASSED
testcases/mount/test_mount_io.py::test_check_io_consistency_premounted[test_dir1] 

PASSED

So, inorder to resolve this, I have added logic to ignore premounted share test, if there are no premounted shares in test-info fie.

This logic to skip is useful even if any user wants to manually run the tests without the value of premounted shares inside the test_info file

[spuiuk]

Please consider the following suggestions for a future PR. These aren't a result of the changes in this PR but will be nice to have for this test.

1. in _run_dbm_consistency_checks() and _check_io_consistency(), the mkdir call should have parents=False. I don't see why we should have this option set to True. A missing parent directory should be treated as an error.
2. _perform_file_operations() should perform the file operations in a sub directory to allow for easier clean up.

[spuiuk]

@spuiuk sit environment is not adding any premounted share value inside test-info created by it:

So, inorder to resolve this, I have added logic to ignore premounted share test, if there are no premounted shares in test-info fie.

This logic to skip is useful even if any user wants to manually run the tests without the value of premounted shares inside the test_info file

This is fine in my opinion. pytest skips the empty tests and it isn't considered an error. I have requested you to remove this logic from the code from my code review which I did earlier.

The premounted shares section is for use with Windows clients where we cannot use mount the shares for the client. In such cases, we need to consider pre-mounted locations to run the tests on. In such cases, we will see the opposite where Skipped messages will be seen on tests which expect to perform the cifs_mounts. eg: test_check_mnt_stress(), test_dbm_consistency(), test_check_io_consistency()

[Shwetha-Acharya]

Please consider the following suggestions for a future PR. These aren't a result of the changes in this PR but will be nice to have for this test.

1. in _run_dbm_consistency_checks() and _check_io_consistency(), the mkdir call should have parents=False. I don't see why we should have this option set to True. A missing parent directory should be treated as an error.
2. _perform_file_operations() should perform the file operations in a sub directory to allow for easier clean up.

I will take a look on it after this PR id merged. Else there can be a lot of merge conflicts.

[spuiuk]

/retest centos-ci/xfs

[spuiuk]

ACK

Confirmed that the CI tests for xfs only fail at smb2.create which is a known failure because of selinux issues. All other tests pass as expected.

[synarete]

lgtm