Skip to content

Add tests for simulating empty body response#182

Open
fraziern wants to merge 1 commit intopretenderjs:masterfrom
fraziern:nullbody
Open

Add tests for simulating empty body response#182
fraziern wants to merge 1 commit intopretenderjs:masterfrom
fraziern:nullbody

Conversation

@fraziern
Copy link

@fraziern fraziern commented Oct 8, 2016

For issue #123 - it appears that null and undefined body responses are already handled OK by pretender. I've added tests for this, and an extra line in the README per the issue comments.

@trek
Copy link
Member

trek commented Oct 11, 2016

@fraziern can you verify that this is what actually happens? My understanding was that XHR responses for no bodies had null (or maybe undefined) response texts, not empty strings?

@fraziern
Copy link
Author

fraziern commented Oct 12, 2016

@trek good question. I did some testing and it looks like request.response is undefined, while request.responseText === ''. So adding strictEqual(request.response, undefined); and strictEqual(request.responseText, ''); to the tests doesn't break them.

If this is causing issues elsewhere, I can take a look, or I can just add the request.response check to the tests.

@trek
Copy link
Member

trek commented Oct 15, 2016

Weird, I just did a small test:

var express = require('express');
var app = express();

app.get('/', function(req, res){
  res.send('<html></html>')
});

app.get('/notfound', function(req, res){
  console.log("1")
  res.status(404).end();
});

app.listen(3001);

/*
  httpRequest = new XMLHttpRequest();
  httpRequest.open('GET', 'http://localhost:3000/404', true);
  httpRequest.send(null);
  httpRequest.onreadystatechange = function(){
    if (httpRequest.readyState === XMLHttpRequest.DONE) {
      debugger;
    }
  }

*/

and both httpRequest.response and httpRequest.responseText are "". What browser did you test in?

@fraziern
Copy link
Author

fraziern commented Oct 16, 2016 •
edited
Loading

I was testing with the existing test framework, rather than a separate setup.

I was able to replicate your results in Chrome. However, if I change the responseType to json first, it returns similar results to what I got using only QUnit asserts (i.e. response is null, responseText is '').

Server:

// app.js
var express = require('express');
var app = express();
var path = require('path');

app.get('/', function(req, res) {
    res.sendFile(path.join(__dirname + '/index.html'));
});


app.get('/save', function(req, res){
  console.log("1");
  res.type('json').status(200).send().end();
});

app.listen(3000, function () {
  console.log('Example app listening on port 3000!');
});

Client:

// index.html
<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title></title>
  </head>
  <body>
Testing 1, 2, 3...
  </body>

  <script type="text/javascript">
  xhr = new XMLHttpRequest();
  xhr.responseType = 'json';  // *** ADDED ***
  xhr.open('GET', 'http://localhost:3000/save', true);
  xhr.send(null);
  xhr.onreadystatechange = function(){
  if (xhr.readyState === XMLHttpRequest.DONE) {
    debugger;
  }
}
  </script>
</html>

Taking a look at the XMLHttpRequest spec, I think this is the correct result:

The response attribute must return the result of running these steps:

If responseType is the empty string or "text"
If state is not loading or done, return the empty string.

Return the text response.

Otherwise
If state is not done, return null.

If responseType is "arraybuffer"
Return the arraybuffer response.

If responseType is "blob"
Return the blob response.

If responseType is "document"
Return the document response.

If responseType is "json"
Return the JSON response.

So I think where that leaves us, is we'd want pretender to properly mimic this behavior, yes? I'm taking a look at this further.

@stefanpenner
Copy link
Contributor

stefanpenner commented Jul 16, 2017

so I think where that leaves us, is we'd want pretender to properly mimic this behavior, yes?

ya, we should mimic spec behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@fraziern fraziern

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fraziern @trek @stefanpenner