Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2749353002:
Add doc on customizing TestRule (Closed)
|
Created:
3 years, 9 months ago by the real yoland Modified:
3 years, 9 months ago CC:
agrieve+watch_chromium.org, chromium-reviews, jbudorick+watch_chromium.org, mikecase+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd doc on customizing TestRule
Review-Url: https://codereview.chromium.org/2749353002
Cr-Commit-Position: refs/heads/master@{#459333}
Committed: https://chromium.googlesource.com/chromium/src/+/d6186299e31a04599eee2e3e2c4dbc9003be40eb
Patch Set 1 : Add doc on customizing TestRule #
Total comments: 10
Patch Set 2 : Address comments #
Total comments: 8
Patch Set 3 : nits #Messages
Total messages: 22 (12 generated)
yolandyan@chromium.org changed reviewers: + jbudorick@chromium.org, mikecase@chromium.org
yolandyan@chromium.org changed reviewers: + nyquist@chromium.org
Patchset #1 (id:1) has been deleted
The CQ bit was checked by yolandyan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... File testing/android/docs/junit4.md (right): https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:136: //2: run any action before @Before nit: suggestion for maybe clearer wording... // 2: Code here runs before @Before // 3. Code here runs after @After
Maybe you'll find the official Google styleguide for Markdown helpful? https://github.com/google/styleguide/blob/gh-pages/docguide/style.md I'm happy to review other style changes to this file after this CL to clean up the rest of the doc if you see errors while doing this CL if you don't want to do that now :-) https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... File testing/android/docs/junit4.md (right): https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:125: ## Customize TestRule examples How about "Customized TestRule example", since it's only one? Also nit: Empty line after header https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:126: TestRule: Nit: Empty line before code block https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:128: public class MyRule implements TestRule { This doesn't need to be indented, since it's already a code block https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:129: //1: add utility methods... Nit: Missing space after // for all these comments. Also, start with uppercase :-)
https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... File testing/android/docs/junit4.md (right): https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:125: ## Customize TestRule examples On 2017/03/16 at 16:15:55, nyquist wrote: > How about "Customized TestRule example", since it's only one? > Also nit: Empty line after header Done https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:126: TestRule: On 2017/03/16 at 16:15:55, nyquist wrote: > Nit: Empty line before code block Done https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:128: public class MyRule implements TestRule { On 2017/03/16 at 16:15:55, nyquist wrote: > This doesn't need to be indented, since it's already a code block Done https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:129: //1: add utility methods... On 2017/03/16 at 16:15:55, nyquist wrote: > Nit: Missing space after // for all these comments. Also, start with uppercase :-) Done https://codereview.chromium.org/2749353002/diff/20001/testing/android/docs/ju... testing/android/docs/junit4.md:136: //2: run any action before @Before On 2017/03/15 at 22:59:24, mikecase wrote: > nit: suggestion for maybe clearer wording... > > // 2: Code here runs before @Before > > // 3. Code here runs after @After Done
+nyquist, ty for the link, I corrected the rest of the doc to align with Google markdown style.
Patchset #2 (id:40001) has been deleted
lgtm https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... File testing/android/docs/junit4.md (right): https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:158: For more, check this [github issue][6] Nit: GitHub https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:160: 2. Use `@UiThreadTest` with caution. Currently, Nit: 1. throughout. https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:169: //Wrong test Nit: space after // and period at the end. Same below. https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:223: - Q: are `@Test` and `@LargeTest/@MediumTest/@SmallTest` annotation both Nit: "Are"
The CQ bit was checked by yolandyan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mikecase@chromium.org, nyquist@chromium.org Link to the patchset: https://codereview.chromium.org/2749353002/#ps80001 (title: "nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... File testing/android/docs/junit4.md (right): https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:158: For more, check this [github issue][6] On 2017/03/23 at 21:28:00, nyquist wrote: > Nit: GitHub Done https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:160: 2. Use `@UiThreadTest` with caution. Currently, On 2017/03/23 at 21:28:00, nyquist wrote: > Nit: 1. throughout. Done https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:169: //Wrong test On 2017/03/23 at 21:28:00, nyquist wrote: > Nit: space after // and period at the end. Same below. Done https://codereview.chromium.org/2749353002/diff/60001/testing/android/docs/ju... testing/android/docs/junit4.md:223: - Q: are `@Test` and `@LargeTest/@MediumTest/@SmallTest` annotation both On 2017/03/23 at 21:28:00, nyquist wrote: > Nit: "Are" Done
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490318881740990,
"parent_rev": "fdd13e6a954714ba0cec7070080eacb057fdb3ff", "commit_rev":
"d6186299e31a04599eee2e3e2c4dbc9003be40eb"}
Message was sent while issue was closed.
Description was changed from ========== Add doc on customizing TestRule ========== to ========== Add doc on customizing TestRule Review-Url: https://codereview.chromium.org/2749353002 Cr-Commit-Position: refs/heads/master@{#459333} Committed: https://chromium.googlesource.com/chromium/src/+/d6186299e31a04599eee2e3e2c4d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as https://chromium.googlesource.com/chromium/src/+/d6186299e31a04599eee2e3e2c4d... |
