jejacks0n / teaspoon

Teaspoon: Javascript test runner for Rails. Use Selenium, BrowserStack, or PhantomJS.
1.43k stars 243 forks source link

Looking for Info on Fixture Cleanup #351

Closed rstudner closed 9 years ago

rstudner commented 9 years ago

So I am writing some tests against code using KnockoutJS.

A key idea of Knockout is "dont apply the bindings twice".

so if I load up an html fixture in beforeEach.. and then have "2 tests". The 2nd test will fail, claiming I already applied the knockout bindings.

This implies that between each test, the fixture/UI isn't actually getting blown away.

Am I missing some small detail (or giant obvious thing haha) in regards to this?

(I can just put if (!appliedBindings) { ko.applyBindings(); appliedBindings = true; } kind of things in all the JS, but that never feels right of course)

to note - the actual code never does "this twice" and all the pages work 100% in general.

mikepack commented 9 years ago

I'm not very familiar with Knockout, but Teaspoon should be cleaning up the fixtures between test runs. Can you paste the test that uses fixtures here? If you're able, it would be extremely helpful if you could create a test app with just Teaspoon, Knockout, and a test that represents the problem you're seeing.

Maybe @jejacks0n has some insight?

rstudner commented 9 years ago

Also - this happens on teaspoon 0.7.9 and 0.9.1 (only two versions i've tried)

mikepack - i'll try to recreate a new project with "way the heck less" to isolate it..

Basically if I run one test (out of the 234) it passes. If I run 2+, then they both/all fail. knockout seems to be quite certain it has "already applied itself" to the fixture.

basically i'll have:

    beforeEach(function(){
      this.fixtures = fixture.load('executive_dashboard/index.html');
    });
    it ("blah") { test some stuff }

so if I do bundle exec teaspoon --filter "just one test" it is fine. once multiple "its" (and therefore beforeEach runs multiple times).. it all blows up.

rstudner commented 9 years ago

Still don't have the new example project, but here is an example test:

//= require support/jasmine-jquery-1.7.0
//= require appliances

describe('appliances', function() {

  beforeEach(function(){
    var available_appliances = {
      appliances: ['ntap'],
      critical: ['ntap'],
      warning: []
    };
    var appliance_data = {
      counts: {
        goodCount: 1,
        warningCount: 0,
        criticalCount: 0,
        totalCount: 1
      },
      data: []
    };
    spyOn($, 'getJSON').andCallFake(function(url,data){
      if(url.match(/ntap.json$/)) {
        data(appliance_data);
      } else {
        data(available_appliances);
      }
    });
    this.fixtures = fixture.load('appliances/index.html');
    PortalAppliances.init();
  });

  it('should display available appliance tab', function() {
    expect($('#ntap').is(':visible')).toBeTrue;
  });

  it('should display available tab status', function(){
    expect($('#ntap-status').is(':visible')).toBeTrue;
  });

  it('should not display the non-available appliance tab', function() {
    expect($('#hx').is(':visible')).toBeFalse;
  });

  it('should update counts', function() {
    expect($('#goodCount').text()).toBe('1');
    expect($('#warningCount').text()).toBe('0');
    expect($('#criticalCount').text()).toBe('0');
    expect($('#totalCount').text()).toBe('1');
  });

});

So, tht PortalAppliances.init() call in beforeEach. That calls ko.applyBindings() on the "fixture".

First test passes, and 2nd subsequest tests all fail, because it is "applying the bindings more than once" which means the fixture isn't truly being 'reset' between tests.

jejacks0n commented 9 years ago

If I were to have to take a guess, it seems like there's nothing resetting the internal references to elements that PortalAppliances has.

Basically, I see an init call, which I'd guess is finding an element and initializing some things with that element, but I don't see anyplace where that element reference is unset, and so without seeing more of your code, are you certain that you're not keeping an internal reference when you don't intend to?

jejacks0n commented 9 years ago

FWIW, I'm like 95% certain that the fixture is in fact being cleared out, and so suggest you move past that as the likely issue you're seeing and investigate further about the internal references that might exist in your implementation logic.

rstudner commented 9 years ago

okay, if 100% of my javascript file is only this:

var PortalAppliances = {
  init: function () {
    ko.applyBindings(
        {
          selectedOrganization: 16,
          goodCount: 0,
          warningCount :0,
          criticalCount: 0,
          applianceLabel: 'foo',
          totalCount: 0,
          tabs: [],
          appliances: []
        }
    );
  }
};

it still fails with the same error. i don't see how it gets any simpler than that :)

Nothing else in the app/this single page etc references that.. only in index.html:

<script type="text/javascript">
  PortalAppliances.init();
</script>

No other references in the entire codebase to it.

jejacks0n commented 9 years ago

I don't see you removing any bindings from anything. does knockout keep an internal reference? so while you say that's all of your code, there's obviously a lot happening in ko that's probably important for you to be aware of.

rstudner commented 9 years ago

Yeah -- I hear what you are saying. I think (think?) this is just a case of using teaspoon/jasmine to test something that capybara/some other framework is meant to test. And -- because knockout didn't freak out about "this" in the old version I was using, once I upgraded, I started seeing it.

mikepack commented 9 years ago

It might be worthwhile to try to clean up the fixtures manually. You can call fixture.cleanup(), but Teaspoon should do this for you automatically. Worth a shot.

rstudner commented 9 years ago

Question - when teaspoon runs from the command line (with phantomJS), does it create even a miniature "DOM" of its own, that it loads the fixture into? (obviously the /teaspoon rails route has a big ole giant DOM of its own)

I think knockout is getting tripped up, because it is somehow remembering "teaspoon DOM" versus JUST the fixture dom (which is being cleaned up correctly I believe).

jejacks0n commented 9 years ago

phantomjs is an implementation of a browser without the UI layer.. part of teaspoon starts this server and then browses to the determined url -- then another part of teaspoon runs within this browser instance. For more clarification you could use something like selenium-webdriver and watch teaspoon run in a browser that's visible. Also, browsing to /teaspoon in your development server exposes what the first part of teaspoon is loading in phantomjs.

You're not taking my advice very well though, which is to say that it's not a fixture problem, it's a cleanup problem. Something is not being released within your application (and to be clear, your application is not just the code that you've written, but is comprised of all of your application dependencies), because how would it? There's no cleanup code that I've seen. A simple way to test my assertion is to try calling PortalAppliances.init() several times within your applications setup script (or wherever this is happening) -- do you not get the same behavior you're seeing in the tests?

If you get the same behavior, then I am correct, if you do not get the same behavior then I might spend more time trying to see what might be broken in regards to the fixtures.

rstudner commented 9 years ago

jejacks0n - no no.. i'm taking your advice 100% well :)

I think wht is going on, is that when someone (me) is saying: ko.applyBindings(jsonObject) without qualifying to what, it is then applying itself to "all of the DOM".. only a part of which (albeit a large part) is the fixgture that is coming and going.

Thus, knockout says: "Wait a second, i've been applied to this global space already"

And yes -- if I call PortalAppliances.init() "twice" in code on the same page, it gets the same error. But in my application, nothing ever calls it twice. but the beforeEach in my test called it "each time" thinking the fixture (i.e. the whole page) was coming/going, each time.

So, what i'm trying, to just wrapping the entire fixture in a div:

<div id="appliancesExperience">
  the page that existed before is just in here wrapped now
</div>

and doing ko.applyBindings(jsonObject, document.getElementById('appliancesExperience'));

This, seems to make it happier.

You comment about fixture cleanup, that I listened to (hah) led me precisely to trying this.

jejacks0n commented 9 years ago

Yup, that's basically it.. so teaspoon does have a view that becomes the "DOM" at the time of browser load -- and the fixture element (where fixtures are loaded into) is part of that. So when you load a fixture, it gets put into the larger dom and does not replace the existing dom.

So basically yes, the root dom doesn't change, so scoping that init to the element that is created, and then removed would fix it. Perfect.