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

library checker tests #258

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lrvideckis
Copy link
Contributor

initial PR just to test that I set it up correctly. I plan to add more tests later

@lrvideckis
Copy link
Contributor Author

can you run the workflow

@simonlindholm
Copy link
Member

Sure. Do I need to set GITHUB_TOKEN/YUKICODER_TOKEN/GH_PAT to something specific (or can they be removed)? I think we might want a stress-tests/librarychecker subdirectory to make the delineation clear.

@lrvideckis
Copy link
Contributor Author

oh good question, hmm, thb I just followed the steps here

https://online-judge-tools.github.io/verification-helper/installer.html

which generated the .yml file

I know that after the tests run, an automated commit is pushed to the repo (test results are stored in a file) so probably the github tokens are needed

@lrvideckis
Copy link
Contributor Author

no you dont need to set them to anything

@lrvideckis
Copy link
Contributor Author

also looks like I broke the other test script; ill move the tests to ~/library-checker e.g. not in stress-tests/...

should fix it

https://github.com/kth-competitive-programming/kactl/blob/main/doc/scripts/run-all.sh#L8

@simonlindholm
Copy link
Member

I know that after the tests run, an automated commit is pushed to the repo (test results are stored in a file) so probably the github tokens are needed

Oh. Is there an option to skip that, or to push it to a separate repo? It sounds spammy

@lrvideckis
Copy link
Contributor Author

lrvideckis commented May 4, 2024

Hi, so I just tested it on my repo, and you only need GITHUB_TOKEN (ill remove the others)

as far as I know, there's no way to prevent having that file. the tool (verification helper) that runs the library checker tests is closely integrated with that file.

for example one annoying thing about verification helper is it only runs 10-15 tests per commit then pushes the results of those tests to that file. (I commented some workarounds). So I think what would happen if you didn't have the file, is only the first 10-15 tests will run, and none others will run

@simonlindholm
Copy link
Member

That sounds ... not great? Can we do this as a thing that's run manually from time to time rather than running it in CI? I imagine most of the value will be in the initial run-through.

@lrvideckis
Copy link
Contributor Author

Oh I should mention, once it runs all tests initially, then afterwards, it only re-runs the tests for files which changed in that commit

@lrvideckis
Copy link
Contributor Author

lrvideckis commented May 4, 2024

but to answer your question, yes, we can set it up where you can run it manually instead of during github CI (then of course anyone can run it manually; locally whenever)

you can follow installation steps here

just FYI For my repo, I have >100 tests which take more than an hour to run

@lrvideckis
Copy link
Contributor Author

lrvideckis commented May 4, 2024

For my repo, the times where verification-helper is annoying is when I do for example some code-formatting change that affects many files, and many tests (more than 15) have to be rerun.

@lrvideckis
Copy link
Contributor Author

@lrvideckis
Copy link
Contributor Author

oh also library checker tests are generated randomly, so you could argue there is value in re-testing more over time (in github CI)

@lrvideckis
Copy link
Contributor Author

lrvideckis commented May 14, 2024

Hi again! let me know if you still want these tests. I'm still willing to write them, but I would like to set it up as part of github CI/actions. I understand your concern about the test file, but consider the pros:

  1. on each new commit, the tests for those changed files are re-run. So the library is continually tested, instead of just "it AC'ed on problem XYZ"
  2. tests are randomly generated, so library gets more tested over time
  3. library checker generates tests to catch specific bugs (to push back against your comment) Basically randomly generating more tests won't necessarily catch certain bugs. One example is kactl uses prufer codes to generate random trees (for example to test LCA). But this results in max expected depth of O(sqrt(n)); meaning this test probably won't catch a TLE if somehow there was a bug resulting in some linear get-lca function.

I can vouch for the quality of these tests (look how many libraries use it). Library checker is maintained by at least 4 codeforces LGM's. Furthermore, specific test cases are crowd-sourced (they already caught one bug in KACTL (in pollard rho))


(opinionated)

If it were up to me, in addition to adding ~/library-checker-tests/ folder, I would also go through the tests in ~/old-unit-tests and ~/stress-tests and convert all those tests into the library checker framework (via using the hello world problem as a "dummy problem"). Then I would delete ~/old-unit-tests, ~/stress-tests, and run-all.sh (which is written in a procedural way; not function as bash is meant to be)

Then I'd remove old implementation of algs like this used only in the current stress-tests

also I would run library checker tests with runtime debug flags to catch index-out-of-bounds; integer overflow, etc

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 this pull request may close these issues.

2 participants