Closed swsnr closed 10 years ago
Sure, I've never used tags for Ert, but it seems pretty straight forward. You or me?
I can give it a shot :)
All right, let me know if you need any assistance, I don't have too much on my table ATM.
I have attached an implementation of this feature, including docs and features, but thanks to a bug in ERT it isn't ready to be merged yet. I need to find a not too hacky way to work around this bug first…
The implementation looks good! I noticed that you already have commit access, so feel free to merge this when you feel it's ready!
I've added a hack around the aforementioned Emacs bug.
It's basically a redefinition of the faulty ert-select-tests
function, which re-implements and fixes the broken part thereof. I couldn't get a defadvice
to work, so I'm now using fset
anyway. I think that's still kind of OK, because being a command line tool we won't run into conflicts with other ERT clients. It's probably even better than an advice, because it's less magical.
Please review this commit again, especially with regards to whether this hack is acceptable. I'd love to see a better, less intrusive workaround, because I am not at all happy with overriding the faulty function, but I couldn't not think of any better way.
Besides, again I wished I could just use Emacs snapshot only. It's new nadvice.el
library would have been the perfect fit here :)
@rejeep Most notably, please also run the tests. I hope that the workaround works as intended, but I am not so confident with regards to older Emacs versions. I'd sleep more easily, if you too tested it before it's merged.
I think that the solution is acceptable, specially because we have tested it. I mean, what can we do...? :)
Two of the tests fails in 24.3.1
:
It seems that the tests run in the opposite order.
Btw, It's really easy to install older versions now that Evm (https://github.com/rejeep/evm) supports pre compiled binaries.
@rejeep Thanks for the heads up. I'll test against Emacs 24.3.1 again then.
@rejeep Well, apparently Emacs 24.3 and Emacs snapshot order tests differently. I don't know how to deal with that… ideas?
I can think of two solutions:
Either using a table (requires some output parsing)
Then I should see output tests:
| name | success |
| this-test | t |
| and-this-test | t |
Or an if statement à la Gherkin :)
Then I should see output if emacs less than "24.4":
"""
passed 1/2 this-test
passed 2/2 and-this-test
"""
And I should see output if emacs more than or equal to "24.4":
"""
passed 1/2 and-this-test
passed 2/2 this-test
"""
@rejeep I'll need new step definitions for these, won't I?
@lunaryorn Yes, but it should be fairly easy to write. Let me know if you want me to
@rejeep I'd be cool, if you could do that :)
Sure, no worries! :)
@rejeep Thank you. Ping we, if you've implemented the steps, I'll fix the tests then.
@lunaryorn I've pushed a fix that works on Emacs 24.3. Not optimal, but it works.
@rejeep Thank you. I tried the tests, and they pass on Emacs snapshot and Emacs 24.3, so I merged this feature.
Great, now I'll just have to start using tags :)
@rejeep Definitely!
@lunaryorn Crap, I always forget this (have to make a test for it). This feature breaks in Emacs 23 because there are some Ert functions that are used before you have the chance to load Ert manually. Can you figure out any smart way of loading this when running?
@rejeep Why can't we load ERT before all the definitions in ert-runner.el
? We surely could delay most ERT use until after ERT is available, but imho it's easier to just make sure that ERT is loaded right away.
We could in ert-runner.el
, but if we do that there no chance for Emacs 23 to load Ert manually before. If you run this in Emacs 23:
$ cask exec ert-runner
The first thing that loads is ert-runner
and so there is no way of doing manual loading of Ert in the test helper, for example like this: https://github.com/rejeep/f.el/blob/master/test/f-init.el#L14-L15
@rejeep And why don't we just add ERT as ert-compat.el
to the ert-runner.el
package, and load if loading ert
fails?
(eval-and-compile
(unless (require 'ert nil 'no-error) (load "ert-compat.el" nil 'no-message)))
We do this in Cask, why don't we use it here?
Sure, we could! That makes a lot of sense.
I will fix that btw :)
ert-runner
already has-p
for filtering by regular expression, so what about-t
to filter by tags?