Skip to content
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

Tests seem to fail randomly? #22

Open
orhanhenrik opened this issue Jan 11, 2016 · 7 comments
Open

Tests seem to fail randomly? #22

orhanhenrik opened this issue Jan 11, 2016 · 7 comments
Labels

Comments

@orhanhenrik
Copy link
Contributor

Hey, I am working on the forceUpdate tests and running into some isses. Two of the tests already written seem to fail at random. Sometimes they work, and sometimes they don't.

These are the errors I am getting:

 1) superagentCache caching tests .get() ._end() should bypass all caching logic:
 Uncaught TypeError: Cannot read property 'body' of undefined
  at null._onTimeout (/Users/orhan/Documents/Work/superagent-cache/test/server/caching.js:62:26)
  at Timer.listOnTimeout (timers.js:92:15)

   1) superagentCache API tests .get() .expiration() .end() should override all caches' defaultExpirations:
 Error: timeout of 2000ms exceeded
  at null.<anonymous> (/Users/orhan/Documents/Work/superagent-cache/node_modules/mocha/lib/runnable.js:165:14)
  at Timer.listOnTimeout (timers.js:92:15)

Are you experiencing this too? Or did I set up something wrong?

Also - do you think it's a better idea to move the express stuff in the tests into a seperate file? Right now it is the same in all files (and has to be, unless you shut down the server in each test it keeps running if you do npm test).

@jpodwys
Copy link
Owner

jpodwys commented Jan 11, 2016

Great idea! I think moving all of the express code to a separate file sounds great.

Those tests are something I've been meaning to address. I'm aware of the intermittent failures. Go ahead and code what you're planning on coding and I'll ignore failures from those two tests when I review your PR.

@orhanhenrik
Copy link
Contributor Author

Also, I have a test like below. the /counter endpoint returns {counter: n} where n is the number of times the endpoint has been requested.

it('.forceUpdate() should prevent the module from hitting the cache', function (done) {
  superagent
    .get('localhost:3000/count')
    .end(function (err, response, key){
      // do a new request to see if it was cached
      superagent
        .get('localhost:3000/count')
        .end(function (err, response, key){
          console.log(response.body.count);
          // the request should ignore the cache and go straight to the server
          expect(response.body.count).toBe(1);
          done();
        });
      });
    });
});

This is where it gets really strange: if I use expect(response.body.count).toBe(1); it works like you would expect. It gets the cached version and the test passes.
However, if I use expect(response.body.count).toBe(2); the callback somehow gets called twice, where the second callback returns a non-cached result (so it actually called the endpoint again rather than using the cache). I have pasted the output below:

With expect(response.body.count).toBe(1);:

◦ .forceUpdate() should prevent the module from hitting the cache: 1
  ✓ .forceUpdate() should prevent the module from hitting the cache

With expect(response.body.count).toBe(2);:

◦ .forceUpdate() should prevent the module from hitting the cache: 1
2
  ✓ .forceUpdate() should prevent the module from hitting the cache (45ms)

This might be an issue with the test setup or something as well. It seems really strange, and I have no idea where to look to find the solution to this.

(PS: I have only created tests, not the feature yet. The tests should fail when the toBe(2) is used until I have created the forceUpdate feature. It does not fail however, like described above.)

You can see the setup here:
https://github.com/orhanhenrik/superagent-cache/commit/c56d2cb3d0e1b0a9139647af4b447f6cd62e0f00

@orhanhenrik
Copy link
Contributor Author

Alright! I will move the express code once I figured out how to get my own tests working.

@jpodwys
Copy link
Owner

jpodwys commented Jan 11, 2016

Thanks! I haven't given much love to those tests recently.

@jpodwys
Copy link
Owner

jpodwys commented Jan 11, 2016

Concerning your tests and the strange behavior you're seeing, I've encountered that sort of thing too. I wouldn't be surprised if my tests are setup poorly and this is a result. If you come up with a clean solution, let me know and I'll be happy to update all the tests. I had a good bit of trouble getting tests with nested calls, and especially nested calls with timeouts as with background refresh, passing consistently.

@jpodwys jpodwys added the bug label Jan 11, 2016
@broncha
Copy link

broncha commented Jun 6, 2019

forceUpdate doesn't seem to be working randomly for me too

@jpodwys
Copy link
Owner

jpodwys commented Jun 6, 2019

Please help me understand. Are you saying the tests fail intermittently, or are you saying the forceUpdate feature is not working?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants