distributed-system-analysis / pbench

A benchmarking and performance analysis framework
http://distributed-system-analysis.github.io/pbench/
GNU General Public License v3.0
188 stars 108 forks source link

Add `pytest-timeout` and refactor `requirements.txt` files #3509

Closed webbnh closed 1 year ago

webbnh commented 1 year ago

After some difficulties I had with a hanging test in #3501, I've decided to add a timeout mechanism to the PyTest tests. This is trivially done simply by adding a requirement for the pytest-timeout plugin and setting a timeout value in the pytest.ini file. Once this change is merged, the PyTest tests will henceforth be aborted if they run longer than 300 seconds (5 minutes). Note that affects only tests run under PyTest, which includes the unit tests and the functional tests but not the Agent "legacy" tests (which, I think, already provide timeouts). Hopefully 5 minutes is long enough for individual tests; if not, we can easily increase the limit.

In the course of making this change, I noted that we have a bunch of redundancy and similar silliness in our Python requirements files for the Agent, Client, and Server, and the "test" ones are referenced (AFAICT) only from the common tox.ini file, so there isn't much need (at the moment, anyway) to maintain strict separation between them since they are always referenced together.

Thus, rather than adding the new pytest-timeout dependency in multiple places, this PR adds it once to a new, top-level test-requirements.txt which contains all the dependencies of the testing system(s) which are common between the Agent, Client, and Server, and it removes the common requirements from the Client- and Server-specific test-requirements.txt files (the Agent-specific test-requirements.txt file ended up empty so I removed it, and so Git regards the creation of the new file as a "rename" of the Agent one), and it fixes up the tox.ini file to match.

[The PR is in Draft mode pending proof that the tests still work under the new organization.]

webbnh commented 1 year ago

If Jenkins is happy, this looks fine.

Mr. Jenkins does indeed appear to be happy! 🎉 (I wasn't sure that each of the functional tests would finish under the limit...that's the most likely sore spot going forward, especially if we put a "large" upload in there...and, I'm still slightly amazed that adding timeouts was as simple as adding a line in a requirements.txt file and adding another line in the pytest.ini file....)

I don't see why you didn't finish the consolidation of test requirements, but that's not a big deal.

I didn't know whether it was important to retain some level of separation or not. I mean, we could just say, "in order to test (any/all of) Pbench, you need the following dependencies" and then specify just a single file at the top level; however, I figured there might be some value in being able to say, "I'm testing just the Agent, so don't bother installing the additional dependencies which only the Server/functional-test requires" (i.e., this is what we currently do for Python 3.6). So, the new test-requirements.txt file is just the common dependencies, and it's used on both P3.6 and P3.9, whereas the additional requirements (of which, currently, only the Server has any) are used only on P3.9...which, hopefully, will save us in the event that the Server develops a dependency on something which isn't available for P3.6.

So...this is now ready for review! 😁