-
Notifications
You must be signed in to change notification settings - Fork 6
improve tests and build CI infrastructure #41
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
Conversation
|
Thanks for your work here, really useful. I will try to review the changes next week. |
|
I rebased this branch on top of the next-release branch |
| - name: test that editorconfig-checker works by letting it output it's version | ||
| run: | | ||
| ec --version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also a kinda weak test, as there's only the exit code that can make the step, thus the workflow, fail.
I remember that for Megalinter, the published binaries for a go tool (probably revive), didn't actually have a version information (maybe limited to the ones released through Docker images). The workaround needed was to build ourselves, like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was intentionally mirroring the make test - if we decide to improve this we should do it in both places at the same time.
run-tests.sh
Outdated
| if [ -n "$TEST_PY_VERSION" ]; then | ||
| PY_VERSIONS+=("$TEST_PY_VERSION") | ||
| else | ||
| PY_VERSIONS+=("2.7") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still runs on Python 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docker tests prove that it does - after all it is just a thin wrapper ;)
run-tests.sh
Outdated
| PY_VERSIONS+=("3.7") | ||
| PY_VERSIONS+=("3.8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two are EOL. Do you still support them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would leave that decision up to @mmicu - I'm fine with keeping EOLed versions as long as there is no technical reason breaking them.
After all editorconfig-checkers place is of a supporting role, maybe helping people slowly transition to newer python versions, and when the only weight for us is some minutes spent running the actions, I do not consider this a cost worth saving. Of cause this argument goes out of the window as soon as we are blocked by a problem that only occurs on EOL versions.
this avoids the need to replace content in the Dockerfile using sed and makes it possible to run a quick test using `make quicktest` (using the defaults for the build arguments specified in the Dockerfile)
This reduces the test runtime from about 5min to 3min on my machine.
… and $TEST_LOCAL_PKG/$TEST_PYPI_PKG
this tests in two ways: - install the package locally in each major os/architecture combination supported by GitHub Actions, then run `editorconfig-checker --version` like `run-tests.sh` does - running `run-tests.sh` once in an ubuntu machines The workflow is triggered both by pushes to any of the branches and for each pull request.
|
@klaernie, thanks for your contribution! Your changes will make our release process much smoother. |
After looking at the recent regression I think it is a good idea to run tests one every change. This way we should no longer accidentally publish a release that does not work for our users.
The tests done are twofold:
run-tests.sh: using the default python installation, install the local package and then runec --versionto ensure a working binary has become available. This is matrixed over the platforms GitHub Actions provides: ubuntu both x64 and arm64, MacOS 13 (the last Intel variant) and 15 (arm64 M1), and lastly x64 Windows (sadly no ARM support there yet).make teston an Ubuntu 24.04 runner, verifying that our backwards compatibility to all python releases is intact.