Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2805133005:
Support running eg tests with xctestrun file on devices. (Closed)
DescriptionSupport running eg tests with xctestrun file on devices.
- Remove dependency on TestProject
- Support xctestrun file
- Support retries of xctests on devices
BUG=708859
Review-Url: https://codereview.chromium.org/2805133005
Cr-Commit-Position: refs/heads/master@{#463326}
Committed: https://chromium.googlesource.com/chromium/src/+/18e8b296007291857d04a633f2ede2907f5ed231
Patch Set 1 #
Total comments: 8
Patch Set 2 : comments #
Total comments: 3
Patch Set 3 : Tuple #
Total comments: 4
Patch Set 4 : test-filter #
Total comments: 1
Patch Set 5 : test-filter #
Messages
Total messages: 21 (6 generated)
huangml@chromium.org changed reviewers: + smut@google.com
PTAL, thanks!
https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/test... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/test... ios/build/bots/scripts/test_runner.py:653: self.xctestrun_file = tempfile.mktemp() mktemp is deprecated, prefer mkstemp. https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/test... ios/build/bots/scripts/test_runner.py:739: {'OnlyTestIdentifiers':['%s' % test_filter]}) 'SkipTestIdentifiers' if invert else 'OnlyTestIdentifiers' https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/xcte... File ios/build/bots/scripts/xctest_utils.py (right): https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/xcte... ios/build/bots/scripts/xctest_utils.py:215: test_name = '%s.%s' % (results.group(1), results.group(2)) Update slash here. https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/xcte... ios/build/bots/scripts/xctest_utils.py:228: test_name = '%s.%s' % (results.group(1), results.group(2)) Also update slash here.
https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/test... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/test... ios/build/bots/scripts/test_runner.py:653: self.xctestrun_file = tempfile.mktemp() On 2017/04/07 21:52:00, smut wrote: > mktemp is deprecated, prefer mkstemp. Done. https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/test... ios/build/bots/scripts/test_runner.py:739: {'OnlyTestIdentifiers':['%s' % test_filter]}) On 2017/04/07 21:52:00, smut wrote: > 'SkipTestIdentifiers' if invert else 'OnlyTestIdentifiers' Done. https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/xcte... File ios/build/bots/scripts/xctest_utils.py (right): https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/xcte... ios/build/bots/scripts/xctest_utils.py:215: test_name = '%s.%s' % (results.group(1), results.group(2)) On 2017/04/07 21:52:00, smut wrote: > Update slash here. Done. https://codereview.chromium.org/2805133005/diff/1/ios/build/bots/scripts/xcte... ios/build/bots/scripts/xctest_utils.py:228: test_name = '%s.%s' % (results.group(1), results.group(2)) On 2017/04/07 21:52:00, smut wrote: > Also update slash here. Done.
https://codereview.chromium.org/2805133005/diff/20001/ios/build/bots/scripts/... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/20001/ios/build/bots/scripts/... ios/build/bots/scripts/test_runner.py:653: self.xctestrun_file = tempfile.mkstemp() mkstemp returns a tuple of file handle, file path. _, self.xctestrun_file = tempfile.mkstemp(). https://docs.python.org/2/library/tempfile.html#tempfile.mkstemp
Patchset #3 (id:40001) has been deleted
Patchset #3 (id:60001) has been deleted
https://codereview.chromium.org/2805133005/diff/20001/ios/build/bots/scripts/... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/20001/ios/build/bots/scripts/... ios/build/bots/scripts/test_runner.py:653: self.xctestrun_file = tempfile.mkstemp() On 2017/04/07 22:31:25, smut wrote: > mkstemp returns a tuple of file handle, file path. _, self.xctestrun_file = > tempfile.mkstemp(). > > https://docs.python.org/2/library/tempfile.html#tempfile.mkstemp Done. Sorry about that, woud tempfile.mkstemp()[1] be safe enough?
I am running some downstream try jobs now with ios_internal/tools/twosided.py (no links because this is public). Will wait on the results to see if it works. https://codereview.chromium.org/2805133005/diff/20001/ios/build/bots/scripts/... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/20001/ios/build/bots/scripts/... ios/build/bots/scripts/test_runner.py:653: self.xctestrun_file = tempfile.mkstemp() On 2017/04/07 22:45:56, huangml1 wrote: > On 2017/04/07 22:31:25, smut wrote: > > mkstemp returns a tuple of file handle, file path. _, self.xctestrun_file = > > tempfile.mkstemp(). > > > > https://docs.python.org/2/library/tempfile.html#tempfile.mkstemp > > Done. Sorry about that, woud tempfile.mkstemp()[1] be safe enough? Sure, fine.
Seeing some successful EG test executions on devices. Lgtm.
https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... ios/build/bots/scripts/test_runner.py:740: {'SkipTestIdentifiers': ['%s' % test_filter]}) Actually, is this right? What is expected format for the value assigned to 'SkipTestIdentifiers'?
https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... ios/build/bots/scripts/test_runner.py:740: {'SkipTestIdentifiers': ['%s' % test_filter]}) On 2017/04/07 23:37:52, smut wrote: > Actually, is this right? What is expected format for the value assigned to > 'SkipTestIdentifiers'? It can accept array of strings, the same as OnlyTestIdentifiers. I assumed that for each failed test, it will call the command once so just put a test_filter inside the [] array. Copied from man xcodebuild.xctesrrun: SkipTestIdentifiers <array of strings> An array of test identifiers that xcodebuild should exclude from the test run. Test Identifier Format Identifiers for both Swift and Objective-C tests are: Test-Class-Name[/Test-Method-Name] To exclude all the tests in a class Example.m, the identifier is just "Example". To exclude one specific test in the class, the identifier is "Example/testExample". OnlyTestIdentifiers <array of strings> An array of test identifiers that xcodebuild should include in the test run. All other tests will be excluded from the test run. The format for the iden- tifiers is described above.
https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... ios/build/bots/scripts/test_runner.py:740: {'SkipTestIdentifiers': ['%s' % test_filter]}) On 2017/04/07 23:44:57, huangml1 wrote: > On 2017/04/07 23:37:52, smut wrote: > > Actually, is this right? What is expected format for the value assigned to > > 'SkipTestIdentifiers'? > > It can accept array of strings, the same as OnlyTestIdentifiers. I assumed that > for each failed test, it will call the command once so just put a test_filter > inside the [] array. > > Copied from man xcodebuild.xctesrrun: > SkipTestIdentifiers <array of strings> > An array of test identifiers that xcodebuild should exclude from the > test run. > > Test Identifier Format > Identifiers for both Swift and Objective-C tests are: > > Test-Class-Name[/Test-Method-Name] > > To exclude all the tests in a class Example.m, the identifier > is just "Example". To exclude one specific test in the class, the identifier is > "Example/testExample". > > OnlyTestIdentifiers <array of strings> > An array of test identifiers that xcodebuild should include in the > test run. All other tests will be excluded from the test run. The format for the > iden- > tifiers is described above. See docstring. test_filter is a list of test cases to filter. So then 'SkipTestIdentifiers': test_filter, not ['%s' % test_filter]. Same for 'OnlyTestIdentifiers'.
https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/80001/ios/build/bots/scripts/... ios/build/bots/scripts/test_runner.py:740: {'SkipTestIdentifiers': ['%s' % test_filter]}) On 2017/04/07 23:46:33, smut wrote: > On 2017/04/07 23:44:57, huangml1 wrote: > > On 2017/04/07 23:37:52, smut wrote: > > > Actually, is this right? What is expected format for the value assigned to > > > 'SkipTestIdentifiers'? > > > > It can accept array of strings, the same as OnlyTestIdentifiers. I assumed > that > > for each failed test, it will call the command once so just put a test_filter > > inside the [] array. > > > > Copied from man xcodebuild.xctesrrun: > > SkipTestIdentifiers <array of strings> > > An array of test identifiers that xcodebuild should exclude from > the > > test run. > > > > Test Identifier Format > > Identifiers for both Swift and Objective-C tests are: > > > > Test-Class-Name[/Test-Method-Name] > > > > To exclude all the tests in a class Example.m, the identifier > > is just "Example". To exclude one specific test in the class, the identifier > is > > "Example/testExample". > > > > OnlyTestIdentifiers <array of strings> > > An array of test identifiers that xcodebuild should include in the > > test run. All other tests will be excluded from the test run. The format for > the > > iden- > > tifiers is described above. > > See docstring. test_filter is a list of test cases to filter. So then > 'SkipTestIdentifiers': test_filter, not ['%s' % test_filter]. > > Same for 'OnlyTestIdentifiers'. Done.
https://codereview.chromium.org/2805133005/diff/100001/ios/build/bots/scripts... File ios/build/bots/scripts/test_runner.py (right): https://codereview.chromium.org/2805133005/diff/100001/ios/build/bots/scripts... ios/build/bots/scripts/test_runner.py:688: # self.install_app() sorry.
lgtm
The CQ bit was checked by huangml@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491845503231230,
"parent_rev": "9a8294176b1ed041294f9c49e232ba2e5de62db1", "commit_rev":
"18e8b296007291857d04a633f2ede2907f5ed231"}
Message was sent while issue was closed.
Description was changed from ========== Support running eg tests with xctestrun file on devices. - Remove dependency on TestProject - Support xctestrun file - Support retries of xctests on devices BUG=708859 ========== to ========== Support running eg tests with xctestrun file on devices. - Remove dependency on TestProject - Support xctestrun file - Support retries of xctests on devices BUG=708859 Review-Url: https://codereview.chromium.org/2805133005 Cr-Commit-Position: refs/heads/master@{#463326} Committed: https://chromium.googlesource.com/chromium/src/+/18e8b296007291857d04a633f2ed... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:120001) as https://chromium.googlesource.com/chromium/src/+/18e8b296007291857d04a633f2ed... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
