Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Compatibility verification / testing #37

Closed
jkerhin opened this issue Dec 9, 2023 · 6 comments · Fixed by #38
Closed

Discussion: Compatibility verification / testing #37

jkerhin opened this issue Dec 9, 2023 · 6 comments · Fixed by #38
Assignees

Comments

@jkerhin
Copy link
Contributor

jkerhin commented Dec 9, 2023

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
Copy link
Owner

ptmcg commented Dec 10, 2023

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 ptmcg self-assigned this Dec 10, 2023
@ptmcg
Copy link
Owner

ptmcg commented Dec 10, 2023

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
Copy link
Contributor Author

jkerhin commented Dec 10, 2023 via email

@ptmcg
Copy link
Owner

ptmcg commented Dec 10, 2023

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

@jkerhin
Copy link
Contributor Author

jkerhin commented Dec 10, 2023

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
Copy link
Owner

ptmcg commented Dec 10, 2023

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!

@ptmcg ptmcg closed this as completed in #38 Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants