ptmcg / logmerger

TUI utility to view multiple log files with merged timeline
MIT License
145 stars 3 forks source link

Discussion: Compatibility verification / testing #37

Closed jkerhin closed 7 months ago

jkerhin commented 7 months ago

Hello!

I spend a lot of time digging through logs at work, and was very excited to hear about logmerger on The Real Python Podcast. I installed logmerger with pipx, but when I tried to use it I got an ImportError. I was going to open an issue, but saw that you had already fixed it in v0.8.0!

Since on that same issue you noted that you were interested in maintaining support for Python 3.9 and up, I put together a couple ways to automatically verify that logmerger is compatible with versions of Python from 3.9 - 3.12. I've got them in the matrix branch on my fork of your repo: link.

At this time, both methods are simple "smoke tests" that just call the CLI help (logmerger -h) and fail if they exit with an error code. I don't have experience testing TUI frameworks, so don't (yet) have good thoughts on how to do more rigorous unit/integration tests.

The GitHub Actions workflow is the easiest to include, and doesn't require you to install anything on your local machine. On each push or pull request, the reusable-test.yml workflow is run, and logmerger -h is run on Python 3.9 - 3.12. These jobs will fail if logmerger exits with a nonzero exit code.

I also added a noxfile.py for use with the nox automation tool. nox requires you to have all the versions of Python already set up on your local machine, but can be run locally without having to push to GitHub. The nox tests are set up with the same logmerger -h "smoke test" that are in the GitHub Actions. I've verified that they do fail with the v0.7.0 release, so even without any more in-depth testing, they would catch e.g. syntax errors.

With v0.7.0:

$ git checkout v0.7.0
$ git checkout matrix -- noxfile.py
$ nox
nox > Running session test-3.9
nox > Creating virtual environment (virtualenv) using python3.9 in .nox/test-3-9
...
nox > Ran multiple sessions:
nox > * test-3.9: failed
nox > * test-3.10: failed
nox > * test-3.11: success
nox > * test-3.12: success

And with v0.8.0:

$ nox
nox > Running session test-3.9
nox > Creating virtual environment (virtualenv) using python3.9 in .nox/test-3-9
...
nox > Ran multiple sessions:
nox > * test-3.9: success
nox > * test-3.10: success
nox > * test-3.11: success
nox > * test-3.12: success

I know this is a bit of a wall of text and introduces a couple tools that you're not currently using. If you're not interested, that's totally OK! But if you're interested in adding a test harness this may be a decent place to start. As I have more time to work with logmerger at work, I may be able provide some more informed feedback and/or pull requests.

Thanks,
Joe

ptmcg commented 7 months ago

This looks great! I've used tox before but not nox. I have a Ubuntu VM with all the Python versions installed that I used for some simple manual version testing for 0.8.0, but some automation is greatly appreciated.

ptmcg commented 7 months ago

I'm not ready to add the Github CI changes but if you could put together a PR with the nox changes, I can merge that into the main branch before I spin up a 0.9.0 dev branch.

jkerhin commented 7 months ago

Ok, sounds good. When I'm at my computer later today I'll open a new PR that's just the noxfile.py

If you'd like I can remove the TODO regarding next steps and/or add a little more to the noxfile docstring about how to get set up with nix.

On Sat, Dec 9, 2023, 23:11 Paul McGuire @.***> wrote:

I'm not ready to add the Github CI changes but if you could put together a PR with the nox changes, I can merge that into the main branch before I spin up a 0.9.0 dev branch.

— Reply to this email directly, view it on GitHub https://github.com/ptmcg/logmerger/issues/37#issuecomment-1848852422, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI3M4SMRFYU7YUKV7BG4AP3YIUY6PAVCNFSM6AAAAABAN5XDNCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYHA2TENBSGI . You are receiving this because you authored the thread.Message ID: @.***>

ptmcg commented 7 months ago

Please just stick to tox for automated testing. I am not looking for any environment or package management integration.

jkerhin commented 7 months ago

Sorry, I didn't see this comment until after I pushed. I'll close the PR and make a new one that uses tox instead of nox.

I won't attempt to add any environment/package management.

ptmcg commented 7 months ago

Did I say tox? Sorry, I use tox at work, so it was muscle memory.

I'm good with learning nox, so don't close that PR yet!