Skip to content

Feat: Add public API for workers#2115

Merged
DavertMik merged 7 commits into
codeceptjs:codeceptjs-v3.0from
KMKoushik:koushik-worker-api
Feb 26, 2020
Merged

Feat: Add public API for workers#2115
DavertMik merged 7 commits into
codeceptjs:codeceptjs-v3.0from
KMKoushik:koushik-worker-api

Conversation

@KMKoushik
Copy link
Copy Markdown
Collaborator

@KMKoushik KMKoushik commented Jan 3, 2020

Motivation/Description of the PR

Applicable helpers:

Type of change

  • Breaking changes
  • New functionality
  • Bug fix
  • Documentation changes/updates
  • Hot fix
  • Markdown files fix - not related to source code

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs)
  • Lint checking (Run npm run lint)
  • Local tests are passed (Run npm test)

@KMKoushik KMKoushik requested a review from DavertMik January 3, 2020 10:39
@KMKoushik
Copy link
Copy Markdown
Collaborator Author

KMKoushik commented Jan 3, 2020

@DavertMik This is just a draft. Kindly review this and we can build over this

Simple Worker

const Workers = require('codeceptjs/lib/workers');

const workers = new Workers(2, false);


workers.run();
//console.log(worker.workers);

workers.on('FAILED', (failedTests, err, stats) => {
  console.log('Failed');
  //console.log('FailedTests :', failedTest);
  console.log('Errors :', err);
  console.log('Stats :', stats);
  for (const failedTest of failedTests) {
    console.log('Title : ', failedTest.title);
    console.log('Message : ', failedTest.err.message)
  }
});

workers.on('PASSED', (stats) => {
  console.log('Success', stats);
});

Complex:

const Workers = require('codeceptjs/lib/workers');

const workers = new Workers();

const testGroups = workers.createGroupsOfTests(2);

const worker1 = workers.spawn();

worker1.addConfig({
  helpers: {
    Puppeteer: {
      url: 'http://localhost',
      show: false
    }
  }
});
worker1.addTests(testGroups[0]);

const worker2 = workers.spawn();

worker2.addConfig({
  helpers: {
    Puppeteer: {
      url: 'http://localhost',
      show: true
    }
  }
});
worker2.addTests(testGroups[1]);

workers.run();

workers.on('FAILED', (failedTests, err, stats) => {
  console.log('Failed');
  //console.log('FailedTests :', failedTest);
  console.log('Errors :', err);
  console.log('Stats :', stats);
  for (const failedTest of failedTests) {
    console.log('Title : ', failedTest.title);
    console.log('Message : ', failedTest.err.message)
  }
});

workers.on('PASSED', (stats) => {
  console.log('Success', stats);
});

Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
@DavertMik
Copy link
Copy Markdown
Contributor

DavertMik commented Jan 4, 2020

My suggestions to usage examples:

const { event } = require('codeceptjs');
const Workers = require('codeceptjs/lib/workers');
const workers = new Workers(2, { by: 'tests' }); // can also be by suite, function etc

workers.run();
//console.log(worker.workers);

workers.on(event.test.failed, (failedTest, err) => { // use standard constants
  console.log('Failed', failedTest);
});

@KMKoushik KMKoushik requested a review from DavertMik January 6, 2020 14:15
@DavertMik DavertMik added the 3.0 label Jan 7, 2020
Copy link
Copy Markdown
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. For shared memory let's make another PR after another round of thinking how to make it right.

Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
@KMKoushik KMKoushik marked this pull request as ready for review January 8, 2020 11:59
DavertMik
DavertMik previously approved these changes Jan 8, 2020
@DavertMik
Copy link
Copy Markdown
Contributor

DavertMik commented Jan 9, 2020

Next: sharing data between master process and workers. Just like we use inject we should add method share - which will add a value to the container and share that values across workers as well.

Usage

share({ userid: 1});

const { userid } = inject();

Usage types:

  • regular run - appends data to container via Container.append().
  • inside master process of workers - sends data to all workers and appends that to container
  • inside worker - adds data locally, sends to master process, and then resends to all workers * (optional)*

In each case, data is appended to a container.

Implementation:

  • share should be global function, which behaves differently depending on a context.
  • share can be a part of container class...
  • when used in workers should be also implemented as a public method inside workers api:
const workers = new Workers(2, { by: 'tests' });
workers.share({ user_id: 2 })

@KMKoushik
Copy link
Copy Markdown
Collaborator Author

Should check for Worker API too. #2141

Copy link
Copy Markdown
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, looks like this design breaks the separation of concerns principle. Container knows too much about workers right now. I don't think we need to store workers inside a container, they need their own container for that :) Maybe we should create a singleton class WorkersStorage and delegate all workers control to it.

So I suggest the following:

1.Create WorkerStorage singleton class in a similar manner as Container - with static methods and top-level variables..

  1. Workers class should use WorkerStorage when it adds a new worker or worker has finished
  2. Add share to WorkerStorage as static method
  3. Container.share should call WorkerStorage.share

Comment thread lib/workers.js Outdated
Comment thread lib/workers.js Outdated
Comment thread lib/container.js Outdated
Comment thread lib/container.js Outdated
@DavertMik
Copy link
Copy Markdown
Contributor

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- test/data/sandbox/custom-worker/share_test.worker.js  1
- lib/workerStorage.js  2
- test/data/sandbox/custom-worker/custom_test.worker.js  1
- test/data/sandbox/codecept.customworker.js  1
- lib/workers.js  7
- test/data/sandbox/custom_worker_helper.js  1
- test/unit/worker_test.js  3
- test/data/sandbox/custom-worker/base_test.worker.js  1
         

Clones added
============
- lib/command/run-workers.js  5
- test/data/sandbox/codecept.customworker.js  1
- lib/workers.js  7
- test/unit/worker_test.js  28
         

See the complete overview on Codacy

@kobenguyent
Copy link
Copy Markdown
Collaborator

@koushikmohan1996 do we support #1967 #1966 for run-workers

@DavertMik
Copy link
Copy Markdown
Contributor

I'm using animated gif first time ever but...

@KMKoushik
Copy link
Copy Markdown
Collaborator Author

@koushikmohan1996 do we support #1967 #1966 for run-workers

This is the public API for workers, I can check those and create a new PR :)

@KMKoushik
Copy link
Copy Markdown
Collaborator Author

Should check for public API too #2155

@gkushang
Copy link
Copy Markdown
Contributor

@koushikmohan1996 @DavertMik

Does it support Gherkin BDD? If so, how?

  • Would it run Features in parallel or Scenarios in parallel?
  • Do users have options to select to run parallel either Features or Scenarios?

@KMKoushik
Copy link
Copy Markdown
Collaborator Author

  • Do users have options to select to run parallel either Features or Scenarios?

@gkushang Yes the option is available
const workers = new Workers(2, { by: 'tests' }) will run tests parallely and new Workers(2, { by: 'suite' }) will split test by suites.
Also, you can supply function to by key to run custom files. check out this example

@gkushang
Copy link
Copy Markdown
Contributor

gkushang commented Feb 5, 2020

@koushikmohan1996 What change I have to make in my existing tests to run these workers in parallel?

I have this file run.js

const { event } = require('codeceptjs');
const Workers = require('codeceptjs/lib/workers');
const workers = new Workers(4, { by: 'suite' }); // can also be by suite, function etc

workers.run();
//console.log(worker.workers);

workers.on(event.test.failed, (failedTest, err) => { // use standard constants
  console.log('Failed', failedTest);
});

When I run node run.js, I runs more threads than tests. I have 3 sceanrios but it runs 9/10 time and fails with below error

(node:95841) UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 127.0.0.1:4444
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1054:14)
(node:95841) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:95841) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:95841) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
(node:95841) UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 127.0.0.1:4444
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1054:14)
(node:95841) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:95841) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 2)
(node:95841) UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 127.0.0.1:4444
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1054:14)

Run with --verbose flag to see NodeJS stacktrace

Run with --verbose flag to see NodeJS stacktrace
(node:95841) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 3)

Let me know if I am doing anything wrong here

@gkushang
Copy link
Copy Markdown
Contributor

gkushang commented Feb 5, 2020

@koushikmohan1996

Here is the example repo if you'd like to try --

https://github.com/gkushang/codeceptjs-bdd/tree/feature/run-workers

STEPS to reproduce:

git clone git@github.com:gkushang/codeceptjs-bdd.git
git fetch origin feature/run-workers:feature/run-workers
git checkout feature/run-workers

cd codeceptjs-bdd/packages/codeceptjs-cucumber/
yarn

node run.js // this runs the workers from this PR

Here is the Workers output,

Workers {
  _events: [Object: null prototype] {},
  _eventsCount: 0,
  _maxListeners: undefined,
  codecept: Codecept {
    config: {
      output: './acceptance/report',
      helpers: [Object],
      include: [Object],
      mocha: {},
      bootstrap: null,
      teardown: null,
      hooks: [],
      gherkin: [Object],
      plugins: [Object],
      cleanup: true,
      dev: [Object],
      name: '<name>',
      multiple: [Object]
    },
    opts: {},
    testFiles: [
      '/Users/kgajjar/Documents/workspace/Salesforce/codeceptjs-bdd/packages/codeceptjs-cucumber/acceptance/features/login/login.feature',
      '/Users/kgajjar/Documents/workspace/Salesforce/codeceptjs-bdd/packages/codeceptjs-cucumber/acceptance/features/search/github-search.feature'
    ]
  },
  finishedTests: {},
  errors: [],
  numberOfWorkers: 4,
  closedWorkers: 0,
  workers: [
    WorkerObject {
      workerIndex: 0,
      options: [Object],
      tests: [Array],
      testRoot: '/Users/kgajjar/Documents/workspace/Salesforce/codeceptjs-bdd/packages/codeceptjs-cucumber'
    },
    WorkerObject {
      workerIndex: 1,
      options: [Object],
      tests: [Array],
      testRoot: '/Users/kgajjar/Documents/workspace/Salesforce/codeceptjs-bdd/packages/codeceptjs-cucumber'
    },
    WorkerObject {
      workerIndex: 2,
      options: [Object],
      tests: [Array],
      testRoot: '/Users/kgajjar/Documents/workspace/Salesforce/codeceptjs-bdd/packages/codeceptjs-cucumber'
    },
    WorkerObject {
      workerIndex: 3,
      options: [Object],
      tests: [],
      testRoot: '/Users/kgajjar/Documents/workspace/Salesforce/codeceptjs-bdd/packages/codeceptjs-cucumber'
    }
  ],
  stats: { passes: 0, failures: 0, tests: 0 },
  testGroups: [
    [ 'SkGIlzX/1jEQNzjnB1MwWw' ],
    [ 'xR6nRjrUgLycYFA7bWgoBg' ],
    [ 'PbEPw3AufxPBZGkTuMNbEQ' ],
    []
  ]
}

Comment thread lib/workers.js
const convertToMochaTests = (testGroup) => {
const group = [];
if (testGroup instanceof Array) {
const mocha = MochaFactory.create({}, {});
Copy link
Copy Markdown
Contributor

@gkushang gkushang Feb 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when Codeceptjs has Gherkin but not mocha?

@KMKoushik
Copy link
Copy Markdown
Collaborator Author

@gkushang On seeing the error Error: connect ECONNREFUSED 127.0.0.1:4444 it seems the selenium is not running. Is it working when using codceptjs run-workers 3

@DavertMik DavertMik changed the base branch from master to codeceptjs-v3.0 February 26, 2020 10:10
@DavertMik DavertMik merged commit e5630e8 into codeceptjs:codeceptjs-v3.0 Feb 26, 2020
Comment thread test/unit/worker_test.js
if (!semver.satisfies(process.version, '>=11.7.0')) this.skip('not for node version');

const workerConfig = {
by: 'test',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need this by.test because in this example you split tests by suites...

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

Labels

Projects

None yet

5 participants