Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

afterEach and after are not being called when a waitsFor times out #18

Merged
merged 3 commits into from

9 participants

@jfirebaugh

I think they should, same as they are when a spec fails outright.

This pull request contains failing specs. I haven't had a chance to investigate a fix.

@jfirebaugh

Hmm, the fix looks tricky. When a waitsFor times out, it sets the abort flag on the block, which causes the queue not to run anything else. In fact we want it to skip only the pending runs, waits or waitsFor, but still run afters.

@infews
Owner

John - I'm finally getting around to cleaning up these old pull requests. What do you want me to do with this one?

@jfirebaugh

Pull it and mark the spec pending maybe? I probably don't have the time to figure out the fix.

I guess it depends on if you agree it's a bug that should be fixed.

@mriddle

Any progress on this? We are also experiencing the same problem. Making TDD difficult :(

@infews
Owner

Sorry for delay on this. A story to investigate this issue is in our backlog here: https://www.pivotaltracker.com/story/show/5515676; we plan on fixing this before 1.2.

@vladev

Is a fix for this in HEAD (I'm observing it with 1.1)?

@ColinCampbell

If not, any news on its implementation? I would love to have this fixed :)

@JustinTulloss

This makes it extremely difficult to prevent one test from affecting the outcome of others. It would be awesome if this could get fixed sooner rather than later.

I looked at it briefly, but it appears that after describing after blocks, they are not marked any differently than regular blocks. Would flagging those blocks on creation and then running them on abort be an acceptable fix?

@brian428

As a side note/addition to this request, could we add an onTimeout arg to waitsFor(), so that you can run a function that can perform cleanup? For me, the one thing I'd like to do is clean up event listeners attached during that spec run.

@rgould

I've expanded on this pull request and modified the Queue system to ensure that afterEach and after blocks are run. See pull request #260

@ragaskar ragaskar merged commit dbcabd3 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 29, 2010
  1. Consolidate all waitsFor specs in the same describe block.

    John Firebaugh authored
  2. Test that afterEach is called after a failing spec.

    John Firebaugh authored
  3. Test that show that afterEach and after are not being called when a w…

    John Firebaugh authored
    …aitsFor times out.
This page is out of date. Refresh to see the latest.
Showing with 130 additions and 84 deletions.
  1. +13 −0 spec/suites/RunnerSpec.js
  2. +117 −84 spec/suites/SpecRunningSpec.js
View
13 spec/suites/RunnerSpec.js
@@ -105,6 +105,19 @@ describe('RunnerTest', function() {
expect(runnerResults.totalCount).toEqual(3);
expect(runnerResults.passedCount).toEqual(3);
});
+
+ it('should run after a failing spec', function () {
+ var afterEach = jasmine.createSpy();
+ env.afterEach(afterEach);
+
+ env.describe('suite', function () {
+ env.it('fails', function () {
+ this.explodes();
+ });
+ }).execute();
+
+ expect(afterEach).toHaveBeenCalled();
+ });
});
View
201 spec/suites/SpecRunningSpec.js
@@ -398,6 +398,123 @@ describe("jasmine spec running", function () {
expect(timeoutSpec.results().getItems()[0].message).toEqual('timeout: timed out after 500 msec waiting for something to happen');
expect(subsequentSpecRan).toEqual(true);
});
+
+ it("runs afterEach after timing out", function() {
+ var afterEach = jasmine.createSpy('afterEach');
+
+ env.describe('foo', function () {
+ env.afterEach(afterEach);
+
+ env.it('waitsFor', function () {
+ this.waitsFor(100, function() {
+ return false;
+ });
+ });
+ }).execute();
+
+ fakeTimer.tick(500);
+ expect(afterEach).toHaveBeenCalled();
+ });
+
+ it("runs single-spec after functions after timing out", function() {
+ var after = jasmine.createSpy('after');
+
+ env.describe('foo', function () {
+ env.it('waitsFor', function () {
+ this.after(after);
+ this.waitsFor(100, function() {
+ return false;
+ });
+ });
+ }).execute();
+
+ fakeTimer.tick(500);
+ expect(after).toHaveBeenCalled();
+ });
+
+ describe('with consecutive calls', function () {
+ var foo;
+ beforeEach(function () {
+ foo = 0;
+ });
+
+ it('exits immediately (does not stack) if the latchFunction times out', function () {
+ var reachedFirstWaitsFor = false;
+ var reachedSecondWaitsFor = false;
+ env.describe('suite that waits', function () {
+ env.it('should stack timeouts', function() {
+ this.waitsFor(500, function () {
+ reachedFirstWaitsFor = true;
+ return false;
+ });
+ this.waitsFor(500, function () {
+ reachedSecondWaitsFor = true;
+ });
+ this.runs(function () {
+ foo++;
+ });
+ });
+ });
+
+ expect(reachedFirstWaitsFor).toEqual(false);
+ env.execute();
+
+ expect(reachedFirstWaitsFor).toEqual(true);
+ expect(foo).toEqual(0);
+ expect(reachedSecondWaitsFor).toEqual(false);
+ fakeTimer.tick(500);
+ expect(reachedSecondWaitsFor).toEqual(false);
+ expect(foo).toEqual(0);
+ fakeTimer.tick(500);
+ expect(reachedSecondWaitsFor).toEqual(false);
+ expect(foo).toEqual(0);
+ });
+
+ it('stacks latchFunctions', function () {
+ var firstWaitsResult = false;
+ var secondWaitsResult = false;
+ var waitsSuite = env.describe('suite that waits', function () {
+ env.it('spec with waitsFors', function() {
+ this.waitsFor(600, function () {
+ fakeTimer.setTimeout(function () {
+ firstWaitsResult = true;
+ }, 300);
+ return firstWaitsResult;
+ });
+ this.waitsFor(600, function () {
+ fakeTimer.setTimeout(function () {
+ secondWaitsResult = true;
+ }, 300);
+ return secondWaitsResult;
+ });
+ this.runs(function () {
+ foo++;
+ });
+ });
+ });
+
+ expect(firstWaitsResult).toEqual(false);
+ expect(secondWaitsResult).toEqual(false);
+ waitsSuite.execute();
+
+ expect(firstWaitsResult).toEqual(false);
+ expect(secondWaitsResult).toEqual(false);
+ expect(foo).toEqual(0);
+
+ fakeTimer.tick(300);
+
+ expect(firstWaitsResult).toEqual(true);
+ expect(secondWaitsResult).toEqual(false);
+ expect(foo).toEqual(0);
+
+ fakeTimer.tick(300);
+
+ expect(firstWaitsResult).toEqual(true);
+ expect(secondWaitsResult).toEqual(true);
+ expect(foo).toEqual(1);
+
+ });
+ });
});
it("testSpecAfter", function() {
@@ -579,90 +696,6 @@ describe("jasmine spec running", function () {
expect(quux).toEqual(1);
});
- describe('#waitsFor should allow consecutive calls', function () {
- var foo;
- beforeEach(function () {
- foo = 0;
- });
-
- it('exits immediately (does not stack) if the latchFunction times out', function () {
- var reachedFirstWaitsFor = false;
- var reachedSecondWaitsFor = false;
- env.describe('suite that waits', function () {
- env.it('should stack timeouts', function() {
- this.waitsFor(500, function () {
- reachedFirstWaitsFor = true;
- return false;
- });
- this.waitsFor(500, function () {
- reachedSecondWaitsFor = true;
- });
- this.runs(function () {
- foo++;
- });
- });
- });
-
- expect(reachedFirstWaitsFor).toEqual(false);
- env.execute();
-
- expect(reachedFirstWaitsFor).toEqual(true);
- expect(foo).toEqual(0);
- expect(reachedSecondWaitsFor).toEqual(false);
- fakeTimer.tick(500);
- expect(reachedSecondWaitsFor).toEqual(false);
- expect(foo).toEqual(0);
- fakeTimer.tick(500);
- expect(reachedSecondWaitsFor).toEqual(false);
- expect(foo).toEqual(0);
- });
-
- it('stacks latchFunctions', function () {
- var firstWaitsResult = false;
- var secondWaitsResult = false;
- var waitsSuite = env.describe('suite that waits', function () {
- env.it('spec with waitsFors', function() {
- this.waitsFor(600, function () {
- fakeTimer.setTimeout(function () {
- firstWaitsResult = true;
- }, 300);
- return firstWaitsResult;
- });
- this.waitsFor(600, function () {
- fakeTimer.setTimeout(function () {
- secondWaitsResult = true;
- }, 300);
- return secondWaitsResult;
- });
- this.runs(function () {
- foo++;
- });
- });
- });
-
- expect(firstWaitsResult).toEqual(false);
- expect(secondWaitsResult).toEqual(false);
- waitsSuite.execute();
-
- expect(firstWaitsResult).toEqual(false);
- expect(secondWaitsResult).toEqual(false);
- expect(foo).toEqual(0);
-
- fakeTimer.tick(300);
-
- expect(firstWaitsResult).toEqual(true);
- expect(secondWaitsResult).toEqual(false);
- expect(foo).toEqual(0);
-
- fakeTimer.tick(300);
-
- expect(firstWaitsResult).toEqual(true);
- expect(secondWaitsResult).toEqual(true);
- expect(foo).toEqual(1);
-
- });
- });
-
it("#beforeEach should be able to eval runs and waits blocks", function () {
var foo = 0;
var bar = 0;
Something went wrong with that request. Please try again.