-
Notifications
You must be signed in to change notification settings - Fork 44
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
WIP: Add support for capturing dynamic binding environment #128
base: master
Are you sure you want to change the base?
Conversation
@@ -675,6 +675,7 @@ See also `buttercup-define-matcher'." | |||
(cl-defstruct (buttercup-suite (:include buttercup-suite-or-spec)) | |||
;; Any children of this suite, both suites and specs | |||
children | |||
environment |
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 probably unnecessary, we can track the environment in the buttercup--current-env
variable (seems like a duplicate)
buttercup.el
Outdated
`((let ,var | ||
,@body)) | ||
body))) | ||
`(buttercup-describe ,description (lambda () ,@new-body) ',env))) |
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.
Passing ',env
here is really ugly... any ideas?
(mapc | ||
(lambda (binding) | ||
(if (consp binding) | ||
(setf (symbol-value (car binding)) (eval (cadr binding))) |
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.
eval
:( But, since these are functions and we are passing data I don't see any good way, other than walking the trees in describe
macro and looking for it
forms and installing let-bindings there.
(funcall body-function)) | ||
(mapc | ||
(lambda (binding) | ||
(setf (symbol-value (car binding)) (cdr binding))) |
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.
setf
might be unnecessary, maybe set
would work the same? (since we are setting the symbol value)
6b3f8ac
to
60b85a3
Compare
Thank you for the PR. Wouldn't it be easier and more readable to just |
@jorgenschaefer Problem with It might work for one What can be done with the Theoretically, this might work (describe "Backup"
:var (kill-ring-backup)
(before-each
(setq kill-ring-backup kill-ring)
(setq kill-ring ("foobar")))
(after-each
(setq kill-ring kill-ring-backup))
(it "kill ring should not be empty"
(expect (< 0 (length kill-ring)) :to-be-truthy))) Then the question is what are the guarantees on |
What you described is a common idiom in javascript but remember that there the scoping works different describe('getProductValue', () => {
let edsnDown
describe('when edsn is up', () => {
beforeEach(() => {
edsnDown = false
})
it("should return '1' when we have elk only", () => {
expect(getProductValue(true, edsnDown)).toEqual('1')
})
it("should return '2' when we have elk and gas", () => {
expect(getProductValue(false, edsnDown)).toEqual('2')
})
})
}) here the |
How about (describe "Backup"
(around-each block
(let ((kill-ring '("foobar")))
(funcall block)))
(it "kill ring should not be empty"
(expect (< 0 (length kill-ring)) :to-be-truthy))) |
@jorgenschaefer That is actually really cool idea. You were doing some ruby lately heh? :D Yea I think that could work if implemented properly. I'll try to take a look on that if you don't mind (or have it already implemented) |
Am I getting a Ruby dialect? Help! :-D I have nothing implemented and I would love such a feature, thank you! |
@Fuco1 Great work so far with this! Looking forward to its completion! P.S. As a Rubyist myself I welcome any suggestion to get buttercup closer to what RSpec offers - it's a pretty amazing framework. |
Is there any progress on this? This is quite an important feature missing in buttercup. Without it, "mocking" special variables becomes quite annoying. |
I probably won't work on this in any reasonably close future. |
This PR is probably older than my maintainership, and as it was marked WIP I never gave it much thought. |
Fixes #127, kind of.
This is quite a bit of black magic and I'm using eval, however I can't really find any other way.
Without using
:env
it falls back to the old code so this should not affect any current usage.A simple test case
without the
:env
flag the second test fails as it is polluted by the first one doing the killing.