Added support for timeout to FakeXMLHttpRequest.

[gordoncl]

This is my attempt to resolve: #3.

I've added a supportsTimeout flag to sinonXhr. When calling send, there is a check to see if the fake timers exist and if so, to listen for ticks through setInterval.

When abort or timeout occur, they follow a lot of the same steps. Only difference really being that abort sets the readyState to UNSENT. For this reason, I've refactored a number of things around abort to keep their behavior the same. This includes refactoring some of the tests.

For more details, see requestErrorSteps and assertRequestErrorSteps.

Some things that you might find questionable are:

1. The way I'm checking to see if the timers are fake. I'm really just checking to see if the clock property exists on the methods I need. This is what fakeServerWithClocks does. I'd be happy to know if there's a more reliable method to test this. It doesn't seem like there is any global clock reference that can be tested against.

2. I think a lot of people might find this syntax to be very gross (https://github.com/Munksey/nise/blob/6c3afbb38a18cf5009a7baed47d9eee8d5f117fd/lib/fake-xhr/index.js#L405). Have no problems changing that to a less condensed if statement.

Let me know if you want me to annotate the PR with github comments. I've added some comments in the code that hopefully explains the rest of my reasoning.

[fatso83]

Thanks for you work. This seems like a good idea, but I am not sure I follow on why there is a restriction on this functionality to faking time with Sinon? Why can't we use this along with normal timeouts? Other than the tests using more time, I don't see why not. Please elaborate on this. Other than that, it seems like a solid piece of work.

[fatso83]

weird wording, but I see another similar test is above it, so I understand you just stuck to the "standard". try changing it to "should set the property if ...", and if you could fix the other one while you are at it, that would be great 🙏

[fatso83]

I don't really understand this restriction. Why would we want to restrict this functionality to fake timers - shouldn't this be up to the programmer if she want to control time?

[gordoncl]

Thanks for taking the time to look at the code. I've rebased to fix the wording on the test.

As for my decision to restrict normal timers, it was mostly a personal decision based on my expectations of how a mocked XMLHttpRequest should behave. My feeling was that if someone mocks XMLHttpRequest for tests, then async behavior is probably not desired.

If we were to always enable timeouts, there might be some backward compatibility issues for some users. Due to performance reasons, they may see different results from their tests depending on their environment (CI or local). Although, this would assume the user is already using non-blocking behavior (like promises) in their tests.

With that said, I'm not sure what the correct behavior should be. I think the argument that it should be up to the user to specify whether timers should or shouldn't be faked at the beginning of their tests is definitely reasonable.

I also find the condition to check if the timers are faked is also somewhat fragile. So I'd say that's another good reason to remove the condition.

Let me know if anything I've said has changed your opinion. If not, I'd be happy to remove the condition and change the tests, accordingly.

[fatso83]

Thanks!