Support native repeat and retry in Mobly. #734
Conversation
* Provides decorators for users to mark test cases for repeat and retry. * Adds new attributes to the TestResultRecord for associating retry test records. * Refactors existing code to support the repeat/retry behavior.
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @xianyuanjia and @xpconanfan)
mobly/base_test.py, line 100 at r1 (raw file):
maximun
nit: typo here, the last char should be 'm'
maximum
mobly/base_test.py, line 637 at r1 (raw file):
def exec_one_test_with_retry(self, test_name, test_method, max_retry_count):
Might need to write the pydoc for this method.
mobly/base_test.py, line 651 at r1 (raw file):
new_record
Do we want to let Mobly user to know the new_record for the retry run?
(ex1: if user want to call BaseTestClass.record_data for recording data for the current test method,
it will overwrite the previous data because each retry they only have the same record)
(ex2: user might want to get the retry number in their test method to print some logs)
If yes, we might need to update BaseTestClass.current_test_info.record?
It might be great if we log some debugging info for each retry round.
mobly/base_test.py, line 655 at r1 (raw file):
return previous_record
Do we really need to return this record?
I don't see the caller side dealing with the return value.
mobly/base_test.py, line 672 at r1 (raw file):
"""
Needs a "Returns" section for the pydoc of exec_one_test.
mobly/records.py, line 333 at r1 (raw file):
self.signature = None self.retry_parent = None
Do we want to update TestResultRecord#to_dict to put the newly created attributes into the test_summary.yaml?
mobly/records.py, line 370 at r1 (raw file):
""" self.begin_time = utils.get_current_epoch_time() self.signature = '%s-%s' % (self.test_name, self.begin_time)
Might need a test case to make sure the self.signature generation logic, for example: test_signature in tests/mobly/records_test.py.
tests/mobly/base_test_test.py, line 2269 at r1 (raw file):
Someting
nit: typo here. should be
Something
There was a problem hiding this comment.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @eric100lin and @xianyuanjia)
mobly/base_test.py, line 637 at r1 (raw file):
Previously, eric100lin (Eric Lin (Tzu Hsiang Lin)) wrote…
def exec_one_test_with_retry(self, test_name, test_method, max_retry_count):Might need to write the pydoc for this method.
Done.
mobly/base_test.py, line 651 at r1 (raw file):
Previously, eric100lin (Eric Lin (Tzu Hsiang Lin)) wrote…
new_recordDo we want to let Mobly user to know the new_record for the retry run?
(ex1: if user want to callBaseTestClass.record_datafor recording data for the current test method,
it will overwrite the previous data because each retry they only have the same record)
(ex2: user might want to get the retry number in their test method to print some logs)
If yes, we might need to updateBaseTestClass.current_test_info.record?It might be great if we log some debugging info for each retry round.
not sure what you mean.
each retry iteration already has its own dedicated record object and entry in the summary yamlright?
mobly/base_test.py, line 655 at r1 (raw file):
Previously, eric100lin (Eric Lin (Tzu Hsiang Lin)) wrote…
return previous_recordDo we really need to return this record?
I don't see the caller side dealing with the return value.
Done.
mobly/base_test.py, line 672 at r1 (raw file):
Previously, eric100lin (Eric Lin (Tzu Hsiang Lin)) wrote…
"""Needs a "Returns" section for the pydoc of
exec_one_test.
Done.
mobly/records.py, line 333 at r1 (raw file):
Previously, eric100lin (Eric Lin (Tzu Hsiang Lin)) wrote…
self.signature = None self.retry_parent = NoneDo we want to update
TestResultRecord#to_dictto put the newly created attributes into the test_summary.yaml?
Good catch! Done.
mobly/records.py, line 370 at r1 (raw file):
Previously, eric100lin (Eric Lin (Tzu Hsiang Lin)) wrote…
Might need a test case to make sure the
self.signaturegeneration logic, for example:test_signaturein tests/mobly/records_test.py.
Done.
There was a problem hiding this comment.
Reviewable status: 3 of 7 files reviewed, 2 unresolved discussions (waiting on @eric100lin, @xianyuanjia, and @xpconanfan)
mobly/base_test.py, line 651 at r1 (raw file):
Previously, xpconanfan (Ang Li) wrote…
not sure what you mean.
each retry iteration already has its own dedicated record object and entry in the summary yamlright?
Oops, you are totally right.
There was a problem hiding this comment.
Reviewable status: 3 of 7 files reviewed, 1 unresolved discussion (waiting on @eric100lin, @xianyuanjia, and @xpconanfan)
mobly/records.py, line 333 at r1 (raw file):
Previously, xpconanfan (Ang Li) wrote…
Good catch! Done.
Sorry, it seems "signature" or "retry_parent" still not in test_summary.yaml.
I wrote a simple script to run with this PR but the results seems no "signature" or "retry_parent":
https://gist.github.com/eric100lin/d6592a5d00e1053a2ab3b2e1636d49c6
… Retry's max_count.
There was a problem hiding this comment.
Reviewable status: 3 of 7 files reviewed, 1 unresolved discussion (waiting on @eric100lin and @xianyuanjia)
mobly/records.py, line 333 at r1 (raw file):
Previously, eric100lin (Eric Lin (Tzu Hsiang Lin)) wrote…
Sorry, it seems "signature" or "retry_parent" still not in test_summary.yaml.
I wrote a simple script to run with this PR but the results seems no "signature" or "retry_parent":
https://gist.github.com/eric100lin/d6592a5d00e1053a2ab3b2e1636d49c6
Done.
There was a problem hiding this comment.
Reviewable status: 3 of 7 files reviewed, 1 unresolved discussion (waiting on @eric100lin and @xianyuanjia)
mobly/records.py, line 333 at r1 (raw file):
Previously, xpconanfan (Ang Li) wrote…
Done.
Done.
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2, 4 of 4 files at r3.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @xianyuanjia)
* Provides decorators for users to mark test cases for repeat and retry. * Adds new attributes to the TestResultRecord for associating retry test records. * Refactors existing code to support the repeat/retry behavior.
This change is