Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate bundles for repeats before minimizing simulator count #431

Closed
wants to merge 0 commits into from

Conversation

RainNapper
Copy link
Contributor

@RainNapper RainNapper commented Apr 7, 2020

Partial resolution of #424

Swap the repeat logic to run before we decide to minimize the simulators to match the bundle size.

If you were running with 10 sims, 1 test, 2 repeats, then the existing logic would cut your sims from 10 to 1, then duplicate bundles from 1 to 2. This left the repeats running in series.

In this change, this would duplicate bundles from 1 to 2, then cut sims from 10 to 2.

Some output of this run:

{84002} 20200407.130243 [  INFO  ] (BLUEPILL) Using xctestrun configuration
{84002} 20200407.130243 [  INFO  ] (BLUEPILL) This is Bluepill v5.2.0-1-g01c5147
{84002} 20200407.130243 [  INFO  ] (BLUEPILL) Packing test bundles based on test counts.
{84002} 20200407.130243 [  INFO  ] (BLUEPILL) Duplicating test bundles 1 time(s) for test repeats
{84002} 20200407.130243 [ WARNING] (BLUEPILL) Lowering number of parallel simulators from 10 to 2 because there aren't enough test bundles.
{84002} 20200407.130243 [  INFO  ] (BLUEPILL) Running with 2 parallel simulators.
{84002} 20200407.130243 [  INFO  ] (BLUEPILL) Packed tests into 2 bundles

Notice the timings indicate they are in fact running in parallel.

{84023} 20200407.130244 [  INFO  ] (BP-1) Running Tests. Attempt Number 1.
{84054} 20200407.130245 [  INFO  ] (BP-2) Running Tests. Attempt Number 1.
{84023} 20200407.130402 [ PASSED ] (BP-1)  50.217322s TestClass/TestName
{84054} 20200407.130406 [ PASSED ] (BP-2)  53.405522s TestClass/TestName

Also verified by running non-headless, and it showed multiple simulators running the same test in parallel.

I was not sure of a great way to test this since I did not see clear points of abstraction, or how to check the number of simulators running in an integration test. Please advise if you feel I should add tests.

@RainNapper RainNapper changed the title Minimize simulator count after duplicating bundles for repeats to all… Duplicating bundles for repeats before minimizing simulator count Apr 7, 2020
@RainNapper RainNapper changed the title Duplicating bundles for repeats before minimizing simulator count Duplicate bundles for repeats before minimizing simulator count Apr 7, 2020
@ravimandala ravimandala self-requested a review April 7, 2020 20:26
if (self.config.cloneSimulator) {
self.testHostSimTemplates = [bpSimulator createSimulatorAndInstallAppWithBundles:xcTestFiles];
if ([self.testHostSimTemplates count] == 0) {
return 1;
}
}

int repeatTestsCount = [self.config.repeatTestsCount intValue];
if (repeatTestsCount > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

With the goal of handling retris inside of bluepill, I think we can move this logic into the main runloop. e.g. When a test fails, simply add repeatTestsCount bundles. Then disable retries in the subprocess

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is not explicitly targeting failure retries. While there might be an option to solve both issues in the same way, this simply enables running a test multiple times, regardless of if the first run passes or fails.

Copy link
Contributor

@ravimandala ravimandala left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add a test or two?

Copy link
Contributor

@ravimandala ravimandala left a comment

Choose a reason for hiding this comment

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

@RainNapper I took another hard look at the changes and I feel the replication should be done early on.

For example, the Packing step would sort the bundles by test count (or test time estimates, if provided). In the case of multiple test bundles being replicated, packing before bundle replication is would not give the sorted bundles as one would expect.

If you fix this, then you can test this the same way we test the packing step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants