Fix tests

ampproject · Jun 2, 2017 · 26eb0b4 · 26eb0b4
1 parent 608c9f7
commit 26eb0b4
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 46 deletions.
diff --git a/build-system/amp4test.js b/build-system/amp4test.js
@@ -31,7 +31,7 @@ app.get('/compose-doc', function(req, res) {
 <body>
 ${req.query.body}
 </body>
-</html>  
+</html>
 `);
 });
 

diff --git a/extensions/amp-a4a/0.1/test/test-amp-a4a.js b/extensions/amp-a4a/0.1/test/test-amp-a4a.js
@@ -2098,8 +2098,11 @@ describe('amp-a4a', () => {
             method: 'GET',
             ampCors: false,
             credentials: 'omit',
-          }).returns(
-          Promise.resolve({keys: [testKey, testKey, testKey]}));
+          }).returns(Promise.resolve({
+            json() {
+              return Promise.resolve({keys: [testKey, testKey, testKey]});
+            },
+          }));
       expect(win.ampA4aValidationKeys).not.to.exist;
       // Key fetch happens on A4A class construction.
       const a4a = new MockA4AImpl(a4aElement);  // eslint-disable-line no-unused-vars
@@ -2136,8 +2139,13 @@ describe('amp-a4a', () => {
             method: 'GET',
             ampCors: false,
             credentials: 'omit',
-          }).returns(
-              Promise.resolve({keys: [JSON.parse(validCSSAmp.publicKey)]}));
+          }).returns(Promise.resolve({
+            json() {
+              return Promise.resolve({keys: [
+                JSON.parse(validCSSAmp.publicKey),
+              ]});
+            },
+          }));
       expect(win.ampA4aValidationKeys).not.to.exist;
       // Key fetch happens on A4A class construction.
       const a4a = new MockA4AImpl(a4aElement);  // eslint-disable-line no-unused-vars
@@ -2168,7 +2176,11 @@ describe('amp-a4a', () => {
             method: 'GET',
             ampCors: false,
             credentials: 'omit',
-          }).returns(Promise.resolve({keys: ['invalid key data']}));
+          }).returns(Promise.resolve({
+            json() {
+              return Promise.resolve({keys: ['invalid key data']});
+            },
+          }));
       expect(win.ampA4aValidationKeys).not.to.exist;
       // Key fetch happens on A4A class construction.
       const a4a = new MockA4AImpl(a4aElement);  // eslint-disable-line no-unused-vars

diff --git a/extensions/amp-access-laterpay/0.1/test/test-amp-access-laterpay.js b/extensions/amp-access-laterpay/0.1/test/test-amp-access-laterpay.js
@@ -131,15 +131,19 @@ describes.fakeWin('LaterpayVendor', {
       xhrMock.expects('fetchJson')
         .withExactArgs('https://builturl', {
           credentials: 'include',
+          requireAmpResponseSourceOrigin: true,
         })
         .returns(Promise.reject({
-          status: 402,
-          json() {
-            return Promise.resolve({access: false});
+          response: {
+            status: 402,
+            json() {
+              return Promise.resolve({access: false});
+            },
           },
         }))
         .once();
       emptyContainerStub.returns(Promise.resolve());
+      debugger;
       return vendor.authorize().then(err => {
         expect(err.access).to.be.false;
       });

diff --git a/extensions/amp-form/0.1/amp-form.js b/extensions/amp-form/0.1/amp-form.js
@@ -412,9 +412,7 @@ export class AmpForm {
         })
         .then(() => this.doXhr_())
         .then(response => this.handleXhrSubmitSuccess_(response),
-            error => this.handleXhrSubmitFailure_(
-                /** @type {!../../../src/service/xhr-impl.FetchError} */ (
-                    error)));
+            error => this.handleXhrSubmitFailure_(error));
 
     if (getMode().test) {
       this.xhrSubmitPromise_ = p;
@@ -488,19 +486,25 @@ export class AmpForm {
 
   /**
    * Transition the form the the submit error state.
-   * @param {../../../src/service/xhr-impl.FetchError} errorResponse
+   * @param {!Error} error
    * @private
    */
-  handleXhrSubmitFailure_(errorResponse) {
-    const error = (errorResponse && errorResponse.error) || errorResponse;
-    this.triggerAction_(
-        /* success */ false, errorResponse ? errorResponse.responseJson : null);
-    this.analyticsEvent_('amp-form-submit-error');
-    this.cleanupRenderedTemplate_();
-    this.setState_(FormState_.SUBMIT_ERROR);
-    this.renderTemplate_(errorResponse.responseJson || {});
-    this.maybeHandleRedirect_(errorResponse.response);
-    user().error(TAG, `Form submission failed: ${error}`);
+  handleXhrSubmitFailure_(error) {
+    let promise;
+    if (error.response) {
+      promise = error.response.json().catch(() => null);
+    } else {
+      promise = Promise.resolve(null);
+    }
+    promise.then(responseJson => {
+      this.triggerAction_(/* success */ false, responseJson);
+      this.analyticsEvent_('amp-form-submit-error');
+      this.cleanupRenderedTemplate_();
+      this.setState_(FormState_.SUBMIT_ERROR);
+      this.renderTemplate_(responseJson || {});
+      this.maybeHandleRedirect_(error.response);
+      user().error(TAG, `Form submission failed: ${error}`);
+    });
   }
 
   /** @private */
@@ -549,7 +553,7 @@ export class AmpForm {
 
   /**
    * Handles response redirect throught the AMP-Redirect-To response header.
-   * @param {!../../../src/service/xhr-impl.FetchResponse} response
+   * @param {../../../src/service/xhr-impl.FetchResponse} response
    * @private
    */
   maybeHandleRedirect_(response) {

diff --git a/extensions/amp-form/0.1/form-verifiers.js b/extensions/amp-form/0.1/form-verifiers.js
@@ -222,11 +222,7 @@ export class AsyncVerifier extends FormVerifier {
     if (this.shouldVerify_()) {
       const xhrConsumeErrors = this.doXhr_().then(() => {
         return [];
-      }, error => {
-        // A 400 error response should be handled here.
-        return getResponseErrorData_(
-            /** @type {!../../../src/service/xhr-impl.FetchError} */ (error));
-      });
+      }, getResponseErrorData_);
 
       const p = this.addToResolver_(xhrConsumeErrors)
           .then(errors => this.verify_(errors))
@@ -411,16 +407,16 @@ class VerificationGroup {
 }
 
 /**
- * @param {!../../../src/service/xhr-impl.FetchError} fetchError
- * @return {!Array<VerificationErrorDef>}
+ * @param {!Error} error
+ * @return {!Promise<!Array<VerificationErrorDef>>}
  * @private
  */
-function getResponseErrorData_(fetchError) {
-  const json = /** @type {?VerificationErrorResponseDef} */ (
-      fetchError && fetchError.responseJson);
-  if (json && json.verifyErrors) {
-    return json.verifyErrors;
-  } else {
-    return [];
+function getResponseErrorData_(error) {
+  const { response } = error;
+  if (!response) {
+    return Promise.resolve([]);
   }
+  return resolve.json().then(json => {
+    return json.verifyErrors || [];
+  }, () => []);
 }
diff --git a/extensions/amp-form/0.1/test/test-amp-form.js b/extensions/amp-form/0.1/test/test-amp-form.js
@@ -37,7 +37,6 @@ import {
   documentInfoForDoc,
   timerFor,
 } from '../../../../src/services';
-import {FetchError} from '../../../../src/service/xhr-impl';
 import '../../../amp-selector/0.1/amp-selector';
 import {toggleExperiment} from '../../../../src/experiments';
 import {user} from '../../../../src/log';
@@ -1493,8 +1492,9 @@ describes.repeated('', {
         json: () => Promise.resolve(),
         headers: headersMock,
       });
-      const fetchRejectPromise = Promise.reject(
-          new FetchError(new Error('Error'), {headers: headersMock}));
+      const error = new Error('Error');
+      error.response = {headers: headersMock};
+      const fetchRejectPromise = Promise.reject(error);
       fetchRejectPromise.catch(() => {
         // Just avoiding a global uncaught promise exception.
       });

diff --git a/src/service/xhr-impl.js b/src/service/xhr-impl.js
@@ -455,6 +455,9 @@ export function assertSuccess(response) {
     const { status } = response;
     const err = user().createError(`HTTP error ${status}`);
     err.retriable = isRetriable(status);
+    // TODO(@jridgewell, #9448): Callers who need the response should
+    // skip processing.
+    err.response = response;
     throw err;
   });
 }
@@ -473,9 +476,12 @@ export class FetchResponse {
     /** @private @const {!XMLHttpRequest|!XDomainRequest} */
     this.xhr_ = xhr;
 
-    /** @type {number} */
+    /** @const {number} */
     this.status = this.xhr_.status;
 
+    /** @const {boolean} */
+    this.ok = this.status >= 200 && this.status < 300;
+
     /** @const {!FetchResponseHeaders} */
     this.headers = new FetchResponseHeaders(xhr);
 

diff --git a/test/functional/test-xhr.js b/test/functional/test-xhr.js
@@ -19,7 +19,6 @@ import {utf8FromArrayBuffer} from '../../extensions/amp-a4a/0.1/amp-a4a';
 import {
   xhrServiceForTesting,
   fetchPolyfill,
-  FetchError,
   FetchResponse,
   assertSuccess,
 } from '../../src/service/xhr-impl';
@@ -294,7 +293,7 @@ describe('XHR', function() {
         it('should reject if error', () => {
           mockXhr.status = 500;
           return assertSuccess(createResponseInstance('', mockXhr))
-              .should.be.rejectedWith(FetchError);
+              .should.be.rejected;
         });
 
         it('should include response in error', () => {

diff --git a/test/integration/test-amp-pixel.js b/test/integration/test-amp-pixel.js
@@ -19,12 +19,11 @@ import {
   withdrawRequest,
 } from '../../testing/test-helper';
 
-describe.only('amp-pixel', () => {
+describe('amp-pixel', () => {
   describes.integration('amp-pixel integration test', {
     body: `<amp-pixel src="${depositRequestUrl('has-referrer')}">`,
   }, env => {
     it('should keep referrer if no referrerpolicy specified', () => {
-      debugger;
       return withdrawRequest(env.win, 'has-referrer').then(request => {
         expect(request.headers.referer).to.be.ok;
       });