Fix tests and implement testing in CI #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes:
tests/purescript-sort-imports-tests.el:31:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
tests/purescript-sort-imports-tests.el:41:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
tests/purescript-sort-imports-tests.el:51:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
tests/purescript-sort-imports-tests.el:61:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
tests/purescript-sort-imports-tests.el:73:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
tests/purescript-sort-imports-tests.el:86:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
tests/purescript-sort-imports-tests.el:99:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
tests/purescript-sort-imports-tests.el:118:14: Warning: ‘goto-line’ is for interactive use only; use ‘forward-line’ instead.
Fixes:
tests/purescript-str-tests.el:1:1: Error: file has no ‘lexical-binding’ directive on its first line
The code that's being removed here made no sense. It is a `check`
rule, and it did the following things:
1. For every `.el` file it was searching its `tests.el` counterpart.
Which doesn't exist.
2. It was checking the correctness of `declare-function`s. Which would
be fine, wasn't it for the fact the project has zero
`declare-function`s.
3. It was checking that `ert` exists, which it does on all supported
Emacs versions.
4. It was removing .elc files before running the tests. Why? 🤷♂️
Replace everything with a single `test` rule which simply loads the
test files and runs the tests.
Besides being actually useful, this also improves running time as:
Initial state | Before | After |
Non-compiled | 2.177 | 1.614 |
Compiled | 2.182 | 0.340 |
|
New commit: Compile .el files at O(1) instead of O(n)
The older code was running Emacs separately for each .el file. Change
the code so that all .el files are passed at once.
This improves build time by x5:
* before: 1.272 sec
* after: 0.257 sec |
The older code was running Emacs separately for each .el file. Change the code so that all .el files are passed at once. This improves build time by x5: * before: 1.272 sec * after: 0.257 sec Also simplify the rule declaration.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The project have had some tests, which even though were apparently from Haskell time, they still seem to be testing some code. The imports sorting probably requires some syntax changes (e.g. to stop accounting for
qualifiedkeyword), but the algo is simple and somebody may still be using it.Now, the interesting part is that the project has
checkMakefile rule (one that I'm removing here). I have no slightest idea what it was written for. As described in the commit, it did the following 4 things:.elfile it was searching itstests.elcounterpart. Which doesn't exist.declare-functions. Which would be fine, wasn't it for the fact the project has zerodeclare-functions.ertexists, which it does on all supported Emacs versions.Replacing
checkwith much simplertestrule which just runs tests, besides being actually useful, also improves running time as follows: