-
Notifications
You must be signed in to change notification settings - Fork 3
Fix tests #62
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
Fix tests #62
Conversation
don't merge this yet, this is just as broken as it was before and I don't really know how to fix it yet, just leaving this here for documentation purposes |
3aca98c
to
e636d86
Compare
alright so the issue is actually fixed now. In order to get everything to work properly we now have additional docker files in the test folders, under normal circumstances we don't actually need or want windows docker images, this is just for the github actions! to make the fix work more smoothly I also changed the CI to not actually execute the script anymore but to just run tests, and added tests that do the same thing as the script invokations (modulo cli parsing). I think this is the better way to do this, when we rework the cli/config parsing stuff I'd then add tests for that as well so we will then have that covered again. note that I needed to rebase this PR onto #68, so it's probably best to merge that first |
Seeing as we move to more of a library structure it seems sensible to separate tests from the script in this proposed way. The fix for the windows tests looks a bit weird, but the resulting bloat seems neglectable and limited to the tests part of the repository. Thank you as always for the changes. |
fixes the windows test that wasn't running properly