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

Ordering test bundles by test execution times read from an input json file #342

Merged
merged 1 commit into from Aug 22, 2019

Conversation

ravimandala
Copy link
Contributor

Introducing an configuration to provide a json file with test case level execution times and subsequently use the execution times to order the test bundles from longest to shortest, for better concurrency. \cc @ob
Ref: #336

Copy link
Member

@ob ob left a comment

Choose a reason for hiding this comment

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

Thanks a lot for picking this up. It'd be awesome if you could add some test cases that demonstrate how this works.

bluepill/src/BPPacker.m Outdated Show resolved Hide resolved
bluepill/src/BPPacker.m Show resolved Hide resolved
bluepill/src/BPPacker.m Outdated Show resolved Hide resolved
@ravimandala
Copy link
Contributor Author

I'll incorporate the comments and add a couple of tests.

bp/src/BPXCTestFile.m Outdated Show resolved Hide resolved
@ravimandala
Copy link
Contributor Author

@oliverhu I have incorporated Oscar and Shawn's comments and added some unit tests. Can you please review? Thanks.

@ravimandala
Copy link
Contributor Author

Same changes are available on #347 but on a different base (tag - v4.1.1).

Copy link
Member

@ob ob left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the previous review, I left some other comments.

bluepill/src/BPPacker.m Outdated Show resolved Hide resolved
bluepill/tests/BPPackerTests.m Outdated Show resolved Hide resolved
bluepill/tests/BPPackerTests.m Outdated Show resolved Hide resolved
bluepill/tests/BPPackerTests.m Outdated Show resolved Hide resolved
bluepill/tests/BPPackerTests.m Outdated Show resolved Hide resolved
@ob
Copy link
Member

ob commented Aug 1, 2019

Oh, one more thing... where is the timings JSON file supposed to come from? I didn't see changes where Bluepill will emit it.

Copy link
Contributor Author

@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.

Incorporated your comments. Please review.

Copy link
Member

@ob ob left a comment

Choose a reason for hiding this comment

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

Looks good. I think the only piece missing is that bluepill should generate the JSON file with the times. After all, it has all the information at the end, when it does the trace-profile.json file...

bp/src/BPConfiguration.m Outdated Show resolved Hide resolved
@ravimandala
Copy link
Contributor Author

Looks good. I think the only piece missing is that Bluepill should generate the JSON file with the times. After all, it has all the information at the end, when it does the trace-profile.json file...

This is an external input to Bluepill and it is not directly derived from any specific thing that Bluepill emits. Based on my experimentation, times from one single execution are NOT reliable estimates for the next execution. In our case, we calculate medians of execution times of a test over the last 7 days.

… file

Introducing an configuration to provide a json file with test case level execution times and subsequently use the execution times to order the test bundles from longest to shortest, for better concurrency.
Also added new tests for the new ordering logic and fixed some existing ones.

Ref: MobileNativeFoundation#336
@ob ob merged commit a6ada58 into MobileNativeFoundation:master Aug 22, 2019
ravimandala added a commit to ravimandala/bluepill that referenced this pull request Apr 3, 2020
… file (MobileNativeFoundation#342)

Introducing an configuration to provide a json file with test case level execution times and subsequently use the execution times to order the test bundles from longest to shortest, for better concurrency.
Also added new tests for the new ordering logic and fixed some existing ones.

Ref: MobileNativeFoundation#336
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.

None yet

3 participants