Skip to content

Introduce pytest-based testing and CI via GitHub Actions#114

Merged
kaifcodec merged 15 commits intokaifcodec:mainfrom
VamatoHD:testing
Dec 22, 2025
Merged

Introduce pytest-based testing and CI via GitHub Actions#114
kaifcodec merged 15 commits intokaifcodec:mainfrom
VamatoHD:testing

Conversation

@VamatoHD
Copy link
Copy Markdown
Collaborator

@VamatoHD VamatoHD commented Dec 20, 2025

The beginning of the implementation of tests. I've made some to test Result and Printer, but they need improving.
Also, I have no clue how to work with GitHub workflows, so @kaifcodec you must do it.

To Do:

  • Add GitHub workflows
  • More tests for the updating logic
  • Tests for orchestrator.py
    • Test if the output is valid JSON and CSV
  • Tests for utils.py
  • Tests for version.py
  • Some automatic way to test each module

@kaifcodec
Copy link
Copy Markdown
Owner

@VamatoHD Ok, no worries, I will look at it when i am free, maybe tomorrow.

@kaifcodec kaifcodec changed the title Add testing Introduce pytest-based testing and CI via GitHub Actions Dec 21, 2025
@kaifcodec
Copy link
Copy Markdown
Owner

@VamatoHD What are the changes in the last commit (d168a04)?

@VamatoHD
Copy link
Copy Markdown
Collaborator Author

@kaifcodec Removed AnyResult type, since Result is fully integrated. Changed monkeytype and telegram return type to be Result. In orchestrator.py, the URL was still being set and is_last had type int when it's a bool.
This way, mypy doesn't error if it gets used to check the code before any PR.

@kaifcodec
Copy link
Copy Markdown
Owner

Oh, got it.

Anyways I added many tests in the recent commits and they are still not ready so I will wait for it and will try to add a CI workflow next.

@VamatoHD
Copy link
Copy Markdown
Collaborator Author

Also, in the new test_orchestrator.py, the line assert any(p.startswith("user") and len(p) > len("user") for p in perms) shouldn't be assert all(p == "user" or (p.startswith("user") and len(p) > len("user")) for p in perms)? To check if all permutations have the prefix of the initial username.

@kaifcodec
Copy link
Copy Markdown
Owner

You're right, all() better matches the function’s contract.
The test enforces the invariant instead of just detecting one valid case.

I wrote all the codes fast and didn't test it for edge cases, that's why it needs more refinements.

@kaifcodec kaifcodec added the enhancement New feature or request label Dec 22, 2025
@kaifcodec kaifcodec self-assigned this Dec 22, 2025
@kaifcodec kaifcodec marked this pull request as ready for review December 22, 2025 12:32
@kaifcodec kaifcodec merged commit abe4d0c into kaifcodec:main Dec 22, 2025
1 check passed
@kaifcodec
Copy link
Copy Markdown
Owner

@VamatoHD I merged this PR earlier to get a better sense of control, but everything looks fine now. I had to do a bit of trial and error since our codebase wasn’t very ruff-friendly, so I made the necessary fixes and it’s working well now.

@VamatoHD VamatoHD deleted the testing branch January 26, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants