search

hapijs / catbox

Multi-strategy object caching service
Other
494 stars 72 forks source link

StaleIn can be function. #105

Closed jardakotesovec closed 9 years ago

jardakotesovec commented 10 years ago

Replaces #94 #96 . Let me know if there is something to tweak. I did not want to get too crazy with tests, so let me know if you also find this as sufficient.

Thanks

jardakotesovec commented 9 years ago

It would be great if this pull request would make it to hapi 8.0. Let me know if there is something I should change/improve.

hueniverse commented 9 years ago

Need to fix the test for and coverage.

jardakotesovec commented 9 years ago

If you run test on catbox Master it will fail occasionally. Likely because of tight timing.. I thought that I identified it in #106, but I just test this adjustment again and it still fails occasionally, so I probably just had lucky row..

Generally relying on 1ms difference in timing seems to be unreliable. I will try to identify which test(s) is causing this.

Is there actually trick how can I see which tests are going through particular line? That would help me to narrow the search.

hueniverse commented 9 years ago

The problem is not timing but problem with the test under the new code module. Look at the specific error.

jardakotesovec commented 9 years ago

Right, this is updated.

With regards to timing reliability. Tests on Master are reliable once I switch off everything (including spotify :) ), with higher cpu load timing gets crazy, which is fair enough. I understand that its about having balance between reliability and tests duration.

This pull request tend to fails occasionally (about 30% failure rate) with

101 tests complete
Test duration: 563 ms
No global variable leaks detected
Coverage: 99.70% (1/335)
lib/policy.js missing coverage on line(s): 120
Code coverage below threshold: 99.70 < 100

Which is when this if is not true:

        if (cached.ttl > 0) {

So I spend half day trying to understand the cause. I thought that typeof is slower or something, but it appeared that even if I just duplicate test (without changing it) on Master, it can cause the failures (I mean this particular failure above). It might be garbage collector that kicks in different time or something like that. I really don't know.

Anyway #106 updates timing for this particular failure, so it works properly. I am basically just trying justify here, that this occasional failure seems to be caused by adding tests, not by tests or code itself. I hope I am not wrong..

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.