README.md: Add details on how the tests are run

[Shwetha-Acharya]

Fixes: #19

[synarete]

Looks fine, but see style comments

[obnoxxx]

thanks for this PR! overall a great effort to enhance the docs! A few detail suggestions though.

[spuiuk]

Hello Shwetha,

Thanks for expanding on this README. This will help new users start out with the sit-test-cases repo and has been required for a while.

My comments are with respect to the sit-environment. While it started out as test cases to be run against the test environment which was setup with sit-environment, we do not have any reason to limit testing to that environment. sit-test-cases is designed to work against any SMB server and will work equally well against a locally running samba server.

I think instead of describing how to setup a sit-environment test infrastructure which requires a lot of resources, we should aim for the easiest option and describe testing against a local SMB server without going into much detail about setting up a Samba server. ie. we should start out with asking the user to have a working SMB server to test against and then proceed to setting up the test-info.yml and running the tests.

We can provide more information for sit-test-cases on sit-environment by either modifying an existing doc or creating a new one in https://github.com/samba-in-kubernetes/sit-environment/tree/main/docs

[Shwetha-Acharya]

Hello Shwetha,

Thanks for expanding on this README. This will help new users start out with the sit-test-cases repo and has been required for a while.

My comments are with respect to the sit-environment. While it started out as test cases to be run against the test environment which was setup with sit-environment, we do not have any reason to limit testing to that environment. sit-test-cases is designed to work against any SMB server and will work equally well against a locally running samba server.

I think instead of describing how to setup a sit-environment test infrastructure which requires a lot of resources, we should aim for the easiest option and describe testing against a local SMB server without going into much detail about setting up a Samba server. ie. we should start out with asking the user to have a working SMB server to test against and then proceed to setting up the test-info.yml and running the tests.

We can provide more information for sit-test-cases on sit-environment by either modifying an existing doc or creating a new one in https://github.com/samba-in-kubernetes/sit-environment/tree/main/docs

I was just wondering if it was good idea to add another option of directly using these tests on already created samba setup in sit-test-cases README.md itself?
If we provide two options, one who does not have samba setup already created can use the automation provided by sit-environment.
I had also not mentioned about already existing samba setup in this doc, as it requires manual efforts. My priority was automation.
But now I agree to your point about the additional resource utilization.

I felt it was necessary to add information in this README.md file because in sit-environment, we are already indicating about sit-test-cases repo.

As sit-test-cases is an individual repo, don't you think adding complete detail here makes it much useful?
Also as a newbie when I had installed this sit-test-cases repo, I was hoping to know about it with the help of README file. However, found the few lines in the existing file not much useful.

Please let me know if you think otherwise @spuiuk

[spuiuk]

I agree that it is important to add information on how to use the repository. Since you are new to this project, your perspective on how best to present this is welcome.

I am assuming that users of this project will already have a SMB server available and would like to run their tests on them. I think it would be very useful if we just get to the point and help users get up and running with sit-test-cases.

We can add a doc on setting up a basic samba server setup for a devel environment for users who want to contribute to the sit-test-cases.

[obnoxxx]

@Shwetha-Acharya ,

Thanks again for your effort to make out docs better and more useful!
I agree with @spuiuk that this README should focus on describing how to run sit-test-cases, against some existing SMB server in the local network. in my opinion, it should not go into details about setting up an SMB server.

while it is perfectly fine and appropriate to mention https://github.com/samba-in-kubernetes/sit-environment, I think this doc and this repo are not the right places to explain how sit-environment works which was one of the main points of my previous review.

information about sit-environment should go into the sit-environment repo's docs. including details about sit-environment in this README doc does not make it more useful, to directly answer your question from the previous comment.

@Shwetha-Acharya , I don't want to sound off-putting. I am sorry if I do! your contribution to improve this doc with first hand experience from trying it out is highly appreciated! Please continue. 😄

[Shwetha-Acharya]

Thanks @obnoxxx and @spuiuk for your review, I will update the next PR accordingly!

[synarete]

Looks fine, but require finial polish.

[spuiuk]

ACK. Thanks for updating the README file.

[spuiuk]

we have a new failure which doesn't seem to have been caused by the change in this PR but we need to fix this ASAP.

From the failed xfs test.

_______________ ERROR collecting testcases/mount/test_mount.py ________________
ImportError while importing test module '/root/sit-test-cases/testcases/mount/test_mount.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
.tox/pytest/lib64/python3.6/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
testcases/mount/test_mount.py:6: in <module>
    import testhelper
E   ModuleNotFoundError: No module named 'testhelper'
__________ ERROR collecting testcases/consistency/test_consistency.py __________
ImportError while importing test module '/root/sit-test-cases/testcases/consistency/test_consistency.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
.tox/pytest/lib64/python3.6/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
testcases/consistency/test_consistency.py:9: in <module>
    import testhelper
E   ModuleNotFoundError: No module named 'testhelper'

[spuiuk]

Just checked the CI run from last night. This passed successfully.
https://jenkins-samba.apps.ocp.cloud.ci.centos.org/job/samba_xfs-integration-test-cases/135/console

Not sure what is causing this new failure.

[spuiuk]

/retest all

[spuiuk]

Shwetha, It looks like your patch is deleting the empty file conftest.py. This is causing issues with the tox running the test and it doesn't seem to accept the PYTHONPATH variable. Can you please fix this as the test is failing.

I get past this problem on my local test machine by simply creating an empty file conftest.py.

[Shwetha-Acharya]

Shwetha, It looks like your patch is deleting the empty file conftest.py. This is causing issues with the tox running the test and it doesn't seem to accept the PYTHONPATH variable. Can you please fix this as the test is failing.

I get past this problem on my local test machine by simply creating an empty file conftest.py.

Done!

[spuiuk]

@anoopcs9 @obnoxxx @synarete @phlogistonjohn can you please take one more look at this PR.

[phlogistonjohn]

Works for me.