Skip to content

Commit

Permalink
Fix closing sync streams in reporters (closes DevExpress#3209) (DevEx…
Browse files Browse the repository at this point in the history
  • Loading branch information
AndreyBelym authored and kirovboris committed Dec 18, 2019
1 parent be27a57 commit d9607ab
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 41 deletions.
12 changes: 7 additions & 5 deletions src/reporter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,20 @@ export default class Reporter {

async dispose () {
if (this.disposed)
return;
return Promise.resolve();

this.disposed = true;

if (!this.outStream || Reporter._isSpecialStream(this.outStream) || !isWritableStream(this.outStream))
return;
return Promise.resolve();

this.outStream.end();

await new Promise(resolve => {
const streamFinishedPromise = new Promise(resolve => {
this.outStream.once('finish', resolve);
this.outStream.once('error', resolve);
});

this.outStream.end();

return streamFinishedPromise;
}
}
37 changes: 31 additions & 6 deletions test/functional/fixtures/reporter/test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
const expect = require('chai').expect;
const { createTestStream, createAsyncTestStream } = require('../../utils/stream');
const fs = require('fs');
const expect = require('chai').expect;
const fs = require('fs');

const {
createSimpleTestStream,
createAsyncTestStream,
createSyncTestStream
} = require('../../utils/stream');

describe('Reporter', () => {
const stdoutWrite = process.stdout.write;
Expand All @@ -12,8 +17,8 @@ describe('Reporter', () => {
});

it('Should support several different reporters for a test run', function () {
const stream1 = createTestStream();
const stream2 = createTestStream();
const stream1 = createSimpleTestStream();
const stream2 = createSimpleTestStream();

return runTests('testcafe-fixtures/index-test.js', 'Simple test', {
only: ['chrome'],
Expand Down Expand Up @@ -183,7 +188,7 @@ describe('Reporter', () => {
});

it('Should support filename as reporter output', () => {
const testStream = createTestStream();
const testStream = createSimpleTestStream();
const reportFileName = 'list.report';

return runTests('testcafe-fixtures/index-test.js', 'Simple test', {
Expand All @@ -207,4 +212,24 @@ describe('Reporter', () => {
fs.unlinkSync(reportFileName);
});
});

it('Should work with streams that emit the "finish" event synchronously (GH-3209)', function () {
const stream = createSyncTestStream();

const runOpts = {
only: ['chrome'],

reporter: [
{
name: 'json',
outStream: stream
}
]
};

return runTests('testcafe-fixtures/index-test.js', 'Simple test', runOpts)
.then(() => {
expect(stream.finalCalled).to.be.ok;
});
});
});
10 changes: 5 additions & 5 deletions test/functional/fixtures/run-options/stop-on-first-fail/test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const expect = require('chai').expect;
const fs = require('fs');
const { createTestStream } = require('../../../utils/stream');
const ReporterPluginHost = require('../../../../../lib/reporter/plugin-host');
const expect = require('chai').expect;
const fs = require('fs');
const { createSimpleTestStream } = require('../../../utils/stream');
const ReporterPluginHost = require('../../../../../lib/reporter/plugin-host');

const TEST_RUN_COUNT_FILENAME = 'testRunCount.txt';

Expand All @@ -28,7 +28,7 @@ describe('Stop test task on first failed test', () => {
});

it('Reporting', () => {
const stream = createTestStream();
const stream = createSimpleTestStream();

return runTests('./testcafe-fixtures/stop-on-first-fail-test.js', void 0, {
shouldFail: true,
Expand Down
30 changes: 15 additions & 15 deletions test/functional/setup.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
const path = require('path');
const Promise = require('pinkie');
const SlConnector = require('saucelabs-connector');
const BsConnector = require('browserstack-connector');
const caller = require('caller');
const promisifyEvent = require('promisify-event');
const createTestCafe = require('../../lib');
const browserProviderPool = require('../../lib/browser/provider/pool');
const BrowserConnection = require('../../lib/browser/connection');
const config = require('./config.js');
const site = require('./site');
const RemoteConnector = require('./remote-connector');
const getTestError = require('./get-test-error.js');
const { createTestStream } = require('./utils/stream');
const path = require('path');
const Promise = require('pinkie');
const SlConnector = require('saucelabs-connector');
const BsConnector = require('browserstack-connector');
const caller = require('caller');
const promisifyEvent = require('promisify-event');
const createTestCafe = require('../../lib');
const browserProviderPool = require('../../lib/browser/provider/pool');
const BrowserConnection = require('../../lib/browser/connection');
const config = require('./config.js');
const site = require('./site');
const RemoteConnector = require('./remote-connector');
const getTestError = require('./get-test-error.js');
const { createSimpleTestStream } = require('./utils/stream');

let testCafe = null;
let browsersInfo = null;
Expand Down Expand Up @@ -174,7 +174,7 @@ before(function () {
global.testCafe = testCafe;

global.runTests = (fixture, testName, opts) => {
const stream = createTestStream();
const stream = createSimpleTestStream();
const runner = testCafe.createRunner();
const fixturePath = typeof fixture !== 'string' ||
path.isAbsolute(fixture) ? fixture : path.join(path.dirname(caller()), fixture);
Expand Down
26 changes: 25 additions & 1 deletion test/functional/utils/stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const { Writable: WritableStream } = require('stream');

const ASYNC_REPORTER_FINALIZING_TIMEOUT = 2000;

module.exports.createTestStream = () => {
module.exports.createSimpleTestStream = () => {
return {
data: '',

Expand Down Expand Up @@ -46,6 +46,30 @@ module.exports.createAsyncTestStream = ({ shouldFail } = {}) => {
});
};

module.exports.createSyncTestStream = () => {
const SyncTestStream = class extends WritableStream {
_callLastArg (args) {
const lastArg = args && args.pop();

if (typeof lastArg === 'function')
lastArg();
}

write (...args) {
this._callLastArg(args);
}

end (...args) {
this.finalCalled = true;

this.emit('finish');
this._callLastArg(args);
}
};

return new SyncTestStream();
};

module.exports.createNullStream = () => {
return {
write: () => {},
Expand Down
14 changes: 7 additions & 7 deletions test/server/legacy-test-run-error-formatting-test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const expect = require('chai').expect;
const read = require('read-file-relative').readSync;
const remove = require('lodash').pull;
const ReporterPluginHost = require('../../lib/reporter/plugin-host');
const testCafeLegacyApi = require('testcafe-legacy-api');
const { createTestStream } = require('../functional/utils/stream');
const expect = require('chai').expect;
const read = require('read-file-relative').readSync;
const remove = require('lodash').pull;
const ReporterPluginHost = require('../../lib/reporter/plugin-host');
const testCafeLegacyApi = require('testcafe-legacy-api');
const { createSimpleTestStream } = require('../functional/utils/stream');

const TEST_RUN_ERROR_TYPE = testCafeLegacyApi.TEST_RUN_ERROR_TYPE;
const LegacyTestRunErrorFormattableAdapter = testCafeLegacyApi.TestRunErrorFormattableAdapter;
Expand All @@ -14,7 +14,7 @@ const userAgentMock = 'Chrome 15.0.874 / Mac OS X 10.8.1';

function assertErrorMessage (file, err, callsite) {
const screenshotPath = '/unix/path/with/<tag>';
const outStreamMock = createTestStream();
const outStreamMock = createSimpleTestStream();
const plugin = new ReporterPluginHost({}, outStreamMock);

const errAdapter = new LegacyTestRunErrorFormattableAdapter(err, {
Expand Down
4 changes: 2 additions & 2 deletions test/server/test-run-error-formatting-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const InvalidElementScreenshotDimensionsError = require('../../lib/err
const SetTestSpeedArgumentError = require('../../lib/errors/test-run').SetTestSpeedArgumentError;
const RoleSwitchInRoleInitializerError = require('../../lib/errors/test-run').RoleSwitchInRoleInitializerError;
const ActionRoleArgumentError = require('../../lib/errors/test-run').ActionRoleArgumentError;
const { createTestStream } = require('../functional/utils/stream');
const { createSimpleTestStream } = require('../functional/utils/stream');

const TEST_FILE_STACK_ENTRY_RE = new RegExp('\\s*\\n?\\(' + escapeRegExp(require.resolve('./data/test-callsite')), 'g');

Expand All @@ -88,7 +88,7 @@ const longSelector = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, s

function assertErrorMessage (file, err) {
const screenshotPath = '/unix/path/with/<tag>';
const outStreamMock = createTestStream();
const outStreamMock = createSimpleTestStream();
const plugin = new ReporterPluginHost({}, outStreamMock);

const errAdapter = new TestRunErrorFormattableAdapter(err, {
Expand Down

0 comments on commit d9607ab

Please sign in to comment.