Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

feat(taskscheduler): allow to use custom scheduler for shards#4836

Open
ReactiveThings wants to merge 2 commits intoangular:masterfrom
ReactiveThings:scheduler
Open

feat(taskscheduler): allow to use custom scheduler for shards#4836
ReactiveThings wants to merge 2 commits intoangular:masterfrom
ReactiveThings:scheduler

Conversation

@ReactiveThings
Copy link
Copy Markdown

When shardTestFiles is true, specs will be sharded by file. (i.e. every test file is executed in separated browser instance).
This behavior can now be customized by providing custom shard scheduler function which allows to run multiple specs in one browser instance. For example we can run same amount of specs in each browser instance.

@psypersky
Copy link
Copy Markdown

psypersky commented Oct 12, 2018

@ReactiveThings could you give me a little bit on how to fix this? I've waiting for this for months so Im going to try to fix it to pass the tests 🙂

@ReactiveThings
Copy link
Copy Markdown
Author

ReactiveThings commented Oct 12, 2018

@psypersky I have fixed tslint issue but it looks like it is not enough. Test "slow asynchronous events waits for http calls" fails for every new pull request. We need to wait until somebody will fix it.

@Gokul-Pulsebeat
Copy link
Copy Markdown

Gokul-Pulsebeat commented Dec 10, 2018

@ReactiveThings Any idea when this feature will be merged? I badly need this feature in my project to speed up the protractor test cases. The performance overhead of starting a new instance of the browser for each spec file offsets the time benefit provided by concurrent execution.

@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

1 similar comment
@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

@Gokul-Pulsebeat
Copy link
Copy Markdown

@ReactiveThings @psypersky - Any movement on this? I am badly waiting for this .

@smi3tana
Copy link
Copy Markdown

Any idea when this feature will be merged?

@Gokul-Pulsebeat
Copy link
Copy Markdown

When will this be merged? Waiting on this for long.

@rayk47
Copy link
Copy Markdown

rayk47 commented Jun 21, 2019

Would love to get this feature in, any headway on it? I reviewed the code and it looks good.

Comment thread spec/unit/taskScheduler_test.js Outdated
@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

When shardTestFiles is true, specs will be sharded by file. (i.e. every test file is executed in separated browser instance).
This behavior can now be customized by providing custom shard scheduler function which allows to run multiple specs in one browser instance. For example we can run same amount of specs in each browser instance.
@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@Gokul-Pulsebeat
Copy link
Copy Markdown

When will this be merged?

@Gokul-Pulsebeat
Copy link
Copy Markdown

Where are we with this PR?

@zChanges
Copy link
Copy Markdown

Why can't feature merge?

@sylvainleduby
Copy link
Copy Markdown

I couldn't wait for this PR to be merged. So, based on elements provided by @ReactiveThings I created a patch in my project (start from the code of the TaskScheduler class constructor of protractor lib version in your project):

custom-task-scheduler.js :

const configParser = require('protractor/built/configParser');
const TaskSchedulerModule = require('protractor/built/taskScheduler');

class CustomTaskScheduler extends TaskSchedulerModule.TaskScheduler {
  constructor(config) {


    // PATCH : prevents multiCapabilities to be handle twice
    super({ ...config, multiCapabilities: [] });


    let excludes = configParser.ConfigParser.resolveFilePatterns(config.exclude, true, config.configDir);
    let allSpecs = configParser.ConfigParser.resolveFilePatterns(
      configParser.ConfigParser.getSpecs(config),
      false,
      config.configDir
    ).filter(path => {
      return excludes.indexOf(path) < 0;
    });
    let taskQueues = [];
    config.multiCapabilities.forEach(capabilities => {
      let capabilitiesSpecs = allSpecs;
      if (capabilities.specs) {
        let capabilitiesSpecificSpecs = configParser.ConfigParser.resolveFilePatterns(
          capabilities.specs,
          false,
          config.configDir
        );
        capabilitiesSpecs = capabilitiesSpecs.concat(capabilitiesSpecificSpecs);
      }
      if (capabilities.exclude) {
        let capabilitiesSpecExcludes = configParser.ConfigParser.resolveFilePatterns(
          capabilities.exclude,
          true,
          config.configDir
        );
        capabilitiesSpecs = capabilitiesSpecs.filter(path => {
          return capabilitiesSpecExcludes.indexOf(path) < 0;
        });
      }


      //
      // PATCH ----------------------------------------------------------------
      //
      // let specLists = [];
      // If we shard, we return an array of one element arrays, each containing
      // the spec file. If we don't shard, we return an one element array
      // containing an array of all the spec files
      // if (capabilities.shardTestFiles) {
      //   capabilitiesSpecs.forEach(spec => {
      //     specLists.push([spec]);
      //   });
      // } else {
      //   specLists.push(capabilitiesSpecs);
      // }
      let shardScheduler = capabilities.shardTestFiles;
      if (typeof shardScheduler !== 'function') {
        // If we shard, we return an array of one element arrays, each containing
        // the spec file. If we don't shard, we return an one element array
        // containing an array of all the spec files
        shardScheduler = function(specs, capabilities) {
          if (capabilities.shardTestFiles) {
            return specs.map(spec => [spec]);
          }
          return [specs];
        };
      }
      const specLists = shardScheduler(capabilitiesSpecs, capabilities);
      // End PATCH ------------------------------------------------------------


      capabilities.count = capabilities.count || 1;
      for (let i = 0; i < capabilities.count; ++i) {
        taskQueues.push(new TaskSchedulerModule.TaskQueue(capabilities, specLists));
      }
    });
    super.taskQueues = taskQueues;
    super.rotationIndex = 0; // Helps suggestions to rotate amongst capabilities
  }
}


// Replace TaskScheduler class in protractor module
TaskSchedulerModule.TaskScheduler = CustomTaskScheduler;

exports.shardScheduler = function shardScheduler(specs, capabilities) {
  const numberOfShards = capabilities.maxInstances;
  if (numberOfShards > 1) {
    const bucketSize = Math.ceil(specs.length / numberOfShards);
    const shards = [];
    let start = 0;
    while (start < specs.length) {
      let end = start + bucketSize;
      if (end > specs.length) {
        end = specs.length;
      }
      shards.push(specs.slice(start, end));
      start = end;
    }
    return shards;
  }
  return [specs];
};

Then require the patch in protractor.conf.js:

const { shardScheduler } = require('<your path to patch>/custom-task-scheduler');

And set shardTestFiles: shardScheduler in capabilities/multiCapabilities (instead of shardTestFiles: true).

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants