Fix: Improve test-server and cdp requests

* Port selection is now random in all cases instead of incremental.
  This should be faster and prevent cases where the platform doesn't
  correctly report if the port is in use.

* The request body in `debugging-protocol-connector.ts` was calcualted
  twice if the request was pending.

* When running serial tests, multiple unnecesary `test-server`s were
  created because they are shared accross tests.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Close #582

package.json

@@ -179,6 +179,7 @@
     "lint": "npm-run-all lint:*",
     "lint:js": "eslint --report-unused-disable-directives --ext md --ext ts --ignore-pattern dist .",
     "lint:md": "markdownlint docs",
+    "nyc": "nyc ava \"dist/tests/lib/rules/cloudinary/**/*.js\" --concurrency=2 --timeout=2m --debug",
     "preparecommitmsg": "node scripts/prepare-commit-message.js",
     "release": "npm i && npm run test && node dist/scripts/release.js",
     "site": "node dist/src/bin/sonar",

src/lib/connectors/debugging-protocol-common/debugging-protocol-connector.ts

@@ -374,11 +374,12 @@ export class Connector implements IConnector {
             return returnValue;
         } catch (e) {
             debug(`Body requested after connection closed for request ${cdpResponse.requestId}`);
-            rawContent = Buffer.alloc(0);
-        }
-        debug(`Content for ${cutString(cdpResponse.response.url)} downloaded`);
+            defaultBody.rawContent = Buffer.alloc(0);
+
+            debug(`Content for ${cutString(cdpResponse.response.url)} downloaded`);
 
-        return defaultBody;
+            return defaultBody;
+        }
     }
 
     /** Returns a Response for the given request. */
@@ -411,6 +412,21 @@ export class Connector implements IConnector {
         const hops: Array<string> = this._redirects.calculate(resourceUrl);
         const originalUrl: string = hops[0] || resourceUrl;
 
+        let eventName: string = this._href === originalUrl ? 'targetfetch::end' : 'fetch::end';
+
+        if (params.type === 'Manifest') {
+            eventName = 'manifestfetch::end';
+        }
+
+        if (eventName !== 'targetfetch::end') {
+            // DOM is not ready so we queue up the event for later
+            if (!this._dom) {
+                this._pendingResponseReceived.push(this.onResponseReceived.bind(this, params));
+
+                return;
+            }
+        }
+
         const response: IResponse = await this.createResponse(params);
 
         const request: IRequest = {
@@ -425,20 +441,7 @@ export class Connector implements IConnector {
             response
         };
 
-        let eventName: string = this._href === originalUrl ? 'targetfetch::end' : 'fetch::end';
-
-        if (params.type === 'Manifest') {
-            eventName = 'manifestfetch::end';
-        }
-
         if (eventName !== 'targetfetch::end') {
-            // DOM is not ready so we queue up the event for later
-            if (!this._dom) {
-                this._pendingResponseReceived.push(this.onResponseReceived.bind(this, params));
-
-                return;
-            }
-
             try {
                 data.element = await this.getElementFromRequest(params.requestId);
             } catch (e) {
@@ -729,7 +732,6 @@ export class Connector implements IConnector {
     private onLoadEventFired(callback: Function): Function {
         return async () => {
             await delay(this._options.waitFor);
-
             const { DOM } = this._client;
             const event: IEvent = { resource: this._finalHref };
 

tests/helpers/rule-runner.ts

@@ -63,6 +63,10 @@ export const testRule = (ruleId: string, ruleTests: Array<IRuleTest>, configs: {
      * a different server and sonar object for each one 
      */
     test.beforeEach(async (t) => {
+        // When running serial tests, the server is shared
+        if (typeof t.context.server !== 'undefined') {
+            return;
+        }
         t.context.server = createServer(configs.https);
         await t.context.server.start();
     });
@@ -141,16 +145,22 @@ export const testRule = (ruleId: string, ruleTests: Array<IRuleTest>, configs: {
                 console.log(`[${connector}] ${ruleTest.name} - try ${attemp}`);
             }
 
-            const { server } = t.context;
-            const { serverUrl, reports } = ruleTest;
-            const target = serverUrl ? serverUrl : `${configs.https ? 'https' : 'http'}://localhost:${server.port}/`;
+            try {
+                const { server } = t.context;
+                const { serverUrl, reports } = ruleTest;
+                const target = serverUrl ? serverUrl : `${configs.https ? 'https' : 'http'}://localhost:${server.port}/`;
+
+                const sonar = await createConnector(t, ruleTest, connector, attemp);
+                const results = await sonar.executeOn(url.parse(target));
 
-            const sonar = await createConnector(t, ruleTest, connector, attemp);
-            const results = await sonar.executeOn(url.parse(target));
+                await stopConnector(ruleTest, sonar);
 
-            await stopConnector(ruleTest, sonar);
+                return validateResults(t, results, reports);
+            } catch (e) {
+                console.error(e);
 
-            return validateResults(t, results, reports);
+                return false;
+            }
         },
         {
             minTimeout: 10000,

tests/helpers/test-server.ts

@@ -140,6 +140,8 @@ export class Server {
 
             // TODO: need to find a way to cast `err` to a [System Error](https://nodejs.org/dist/latest-v7.x/docs/api/errors.html#errors_system_errors)
             this._server.on('error', (err: any) => {
+                console.error(err.code);
+
                 if (err.code === 'EADDRINUSE' || err.code === 'EACCES') {
                     setImmediate(() => {
                         this._port++;
@@ -154,7 +156,10 @@ export class Server {
                 }
             });
 
-            this._server.once('listening', resolve);
+            this._server.once('listening', () => {
+                console.log(`Listening on ${this._port}`);
+                resolve();
+            });
 
             this._server.listen(this._port);
         });

tests/lib/rules/image-optimization-cloudinary/tests.ts

@@ -65,27 +65,26 @@ const mockCloudinary = (responses) => {
     mock('cloudinary', mockedModule);
 };
 
-
 const tests: Array<IRuleTest> = [
     {
         before() {
-            mockCloudinary(noSavings);
+            mockCloudinary(savings50);
         },
-        name: 'optimized SVG',
+        name: 'unoptimized PNG',
+        reports: [{ message: `File http://localhost/nellie-studying.png could be around 143.62kB (50%) smaller.` }],
         serverConfig: {
-            '/': generateHTMLPage('', `<img src="space-nellie.svg">`),
-            '/space-nellie.svg': generateImageData(svg, 'image/svg+xml')
+            '/': generateHTMLPage('', `<img src="nellie-studying.png">`),
+            '/nellie-studying.png': generateImageData(png, 'image/png')
         }
     },
     {
         before() {
-            mockCloudinary(savings50);
+            mockCloudinary(noSavings);
         },
-        name: 'unoptimized PNG',
-        reports: [{ message: `File http://localhost/nellie-studying.png could be around 143.62kB (50%) smaller.` }],
+        name: 'optimized SVG',
         serverConfig: {
-            '/': generateHTMLPage('', `<img src="nellie-studying.png">`),
-            '/nellie-studying.png': generateImageData(png, 'image/png')
+            '/': generateHTMLPage('', `<img src="space-nellie.svg">`),
+            '/space-nellie.svg': generateImageData(svg, 'image/svg+xml')
         }
     }
 ];
@@ -116,11 +115,13 @@ const testThresholds: Array<IRuleTest> = [
 ];
 
 ruleRunner.testRule(ruleName, testThresholds, {
+    ignoredConnectors: ['chrome'],
     ruleOptions: { apiKey: 'fakeApiName', apiSecret: 'fakeApiSecret', cloudName: 'fakeCloudName', threshold: 150 },
     serial: true
 });
 
 ruleRunner.testRule(ruleName, tests, {
+    ignoredConnectors: ['chrome'],
     ruleOptions: { apiKey: 'fakeApiName', apiSecret: 'fakeApiSecret', cloudName: 'fakeCloudName' },
     serial: true
 });