Context mutations in it blocks leak into sibling describe blocks
#2914
Comments
sleexyz
changed the title
describe blocks break test isolation.it blocks leak into sibling describe blocks
|
(TL;DR at/after the "what would be particularly helpful" line) If I recall correct from the last time I looked into describe("asdf", function () {
describe("inner 1", function () {
it("works", function () {
if (foo) {
throw new Error('foo should not exist');
}
});
});
var foo
it("setting foo", function () {
foo = true;
});
});- describe("asdf", function () {
var foo
beforeEach(function () {
foo = false;
});
describe("inner", function () {
describe("superinner 1", function () {
it("works", function () {
if (foo) {
throw new Error('foo should not exist');
}
});
});
var foo
it("setting foo", function () {
foo = true;
});
});
});- describe("asdf", function () {
var foo
beforeEach(function () {
foo = false;
});
describe("inner 1", function () {
it("works", function () {
if (foo) {
throw new Error('foo should not exist');
}
});
});
it("setting foo", function () {
foo = true;
});
});I'm not saying this is a good design -- if nothing else it seems (almost) entirely redundant given you can achieve the same behavior without What would be particularly helpful to us is:
(I've actually looked at a proposal earlier this year about either changing the behavior of |
|
Hi @ScottFreeCode. Here's an idea of my proposed spec, of which we limit for the sake of explanation to just the semantics of Invariants:
Hypothetical implementation notes:When a test suite is executed, each test should run its constituent code blocks in the following order:
Here are some advantages from those core principles:
I took a look at # 1. tests should not leak to sibling describe blocks - Pass (expected: Pass) ✔
describe "asdf" do
describe "inner 1" do
it "works" do
expect(defined? @foo).to eql nil
end
end
it "(mutation)" do
@foo = true
end
end
# 2. tests should not leak to sibling describe blocks when a higher beforeEach exists - Pass (expected: Pass) ✔
describe "asdf" do
before(:each) do
@foo = true
end
describe "inner 1" do
describe "superinner 1" do
it "works" do
expect(@foo).to be true
end
end
it "setting foo" do
@foo = false
end
end
end
# 3. tests should not leak to sibling describe blocks when a sibling beforeEach exists - Pass (expected: Pass) ✔
describe "asdf" do
before(:each) do
@foo = true
end
describe "inner 1" do
it "works" do
expect(@foo).to be true
end
end
it "setting foo" do
@foo = false
end
endIn other words, |
|
Here's another example of the test isolation I'm looking for in other testing frameworks in other languages, this time in Test should be isolated from other tests - Pass (expected: Pass) ✅#!/usr/bin/env stack
-- stack --resolver lts-8.0 --install-ghc runghc --package hspec -- -Wall -Werror
import Test.Hspec
import Data.IORef
main :: IO ()
main = hspec $ do
describe "asdf" $ beforeWith (\() -> newIORef False) $ do
describe "inner" $ beforeWith (\ctx -> writeIORef ctx True >> return ctx) $ do
it "ctx should be true" $ \ctx -> do
readIORef ctx >>= shouldBe True
it "(setting ctx to False)" $ \ctx -> do
writeIORef ctx FalseIn the equivalent
|
|
Good to know RSpec's behavior here; thanks! This might not be relevant in the case of Haskell with its automatic memoization and lack of mutation (and I apologize if it was mentioned already and I missed it), but does RSpec have a I'd also be interested in figuring out the behavior of |
|
One other proposal I believe we considered not too long ago was (off the top of my head; I don't recall the exact details but I think this was the gist of it) to make var assert = require("assert")
var configurableType = require("./configurable-type")
describe("parameterized behavior", function() {
before(function() {
this.configuration = 42
})
beforeEach(function() {
this.configurableObject = configurableType(this.configuration)
})
it("should be configured for 42", function() {
assert.equals(this.configurableObject.configuration, 42) // imagine testing more complex behavior that depends on the object's parameter
})
describe("with different parameter", function() {
before(function() {
this.configuration = 7
})
it("should be configured for 7", function() {
assert.equals(this.configurableObject.configuration, 7) // imagine testing more complex behavior that depends on the object's parameter
})
})
})Or at any rate, it was some kind of proposal to allow a resource defined in an outer suite to have some kind of dependency defined alongside it in the outer suite and then overridden in an inner suite. I think it was also RSpec-inspired, although, and this is purely my fault, I spent a lot of the discussion accidentally figuring out that Mocha's current |
|
Experiencing a similar problem, having describe("A broken mocha", function() {
beforeEach(function() {
this.variable = "foo"
})
describe("A nested context", function() {
beforeEach(function() {
expect(this.variable).to.deep.equal("foo")
this.variable = "bar"
})
it("should", function() {
expect(this.variable).to.deep.equal("bar")
})
it("should", function() {
expect(this.variable).to.deep.equal("bar")
})
})
})Removing the nested describe causes things to work correctly. |
|
this seems similar to some other bug I commented on the other day. use closures instead of context object |
|
@boneskull That is the decision Jest goes with; they remove the context object entirely. We switched over to Jest from mocha, and initially missed mocha's context object. We then switched over to using closures, which was slightly syntactically uglier, since you'd have to first declare your variables uninitialized, and then mutate them in In any case, we found that both solutions were both lacking in terms of flow type inference, so we ended up settling on a third solution: functions. i.e. function setupTest (params: { url: string }): { foo: string, bar: string } {
...
}
it("blah", () => {
const { foo, bar } = setupTest({ url: "https://github.com" });
...
});It's an extra line of code for each test, but you get type inference on both your inputs and outputs. |
|
Can anyone weigh in on whether this issue is distinct from #2014 (see also my summary at #2014 (comment))? I've been trying to close some duplicates lately. (We may also want to see if we can make that one more likely to show up in searches related to any variation on, or interpretation of, this issue.) |
|
@ScottFreeCode this seems to be a duplicate of #2014. closing |
When a
describeblock has innerdescribeblocks, as well as immediateitblocks, thethiscontext of the immediateitblocks leak to the tests in the innerdescribeblocks.In addition, when a
describeblock has innerdescribeblocks, as well as immediateitblocks, all in the context of some parent describe block with abeforeEach, thethiscontext of the immediateitblocks leak to the tests in the innerdescribeblocks, overriding the top levelbeforeEach!Interestingly, when
beforeEachis sibling to theitblock and the innerdescribeblock, mocha works as expected: the context mutation doesn't override thebeforeEach.The text was updated successfully, but these errors were encountered: