Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2084093003:
Add input dependencies on installer test scripts (Closed)
DescriptionAdd input dependencies on installer test scripts
This will cause recompiles when the scripts change, forcing the bots to
run the tests. It replaces GYP's test_installer_sentinel target.
BUG=621988
Committed: https://crrev.com/ca63fc255102c88323389f36b47d24e7063c4c15
Cr-Commit-Position: refs/heads/master@{#401426}
Patch Set 1 #
Total comments: 1
Patch Set 2 : typo #
Messages
Total messages: 26 (11 generated)
brettw@chromium.org changed reviewers: + brucedawson@chromium.org
This is easy now that inputs force recompiles (this is a new change in GN).
The CQ bit was checked by brettw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2084093003/1
One typo, but also while this change looks good I cannot prove that it works. I applied the patch locally and tried this sequence: ninja -v -C out\gn_release_component installer_util_unittests setup_unittests upgrade_test mini_installer touch chrome\test\mini_installer\verifier.py ninja -v -C out\gn_release_component installer_util_unittests setup_unittests upgrade_test mini_installer The second build does "stamp obj/chrome/installer/mini_installer/lib.inputdeps.stamp" but nothing else - no compiles. I'm not sure why not because the dependencies seem to be there. https://codereview.chromium.org/2084093003/diff/1/chrome/installer/mini_insta... File chrome/installer/mini_installer/BUILD.gn (right): https://codereview.chromium.org/2084093003/diff/1/chrome/installer/mini_insta... chrome/installer/mini_installer/BUILD.gn:46: # dependency because a depedency won't actually trigger recompiles in depedency->dependency
I tested it by compiling chrome/test/mini_installer:lib, touching a test file, and it did recompile the lib target. My GN version is 400967. If you have an older one it won't have any effect. Could that be it?
On 2016/06/21 20:11:19, brettw wrote: > I tested it by compiling chrome/test/mini_installer:lib, touching a test file, > and it did recompile the lib target. > > My GN version is 400967. If you have an older one it won't have any effect. > Could that be it? I am on 398658, so that must be it. I'll pull to latest and try again later.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by brettw@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2084093003/1
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
grt@chromium.org changed reviewers: + grt@chromium.org
rubberstamp OWNERS lgtm. i trust you two. thanks for fixing this.
The CQ bit was checked by brettw@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2084093003/1
I pulled to latest and now my test scenario behaves as desired - seven source files recompile when I touch one of the test inputs. Please fix the comment typo and then lgtm
The CQ bit was unchecked by brettw@chromium.org
typo
The CQ bit was checked by brettw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from brucedawson@chromium.org, grt@chromium.org Link to the patchset: https://codereview.chromium.org/2084093003/#ps20001 (title: "typo")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2084093003/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add input dependencies on installer test scripts This will cause recompiles when the scripts change, forcing the bots to run the tests. It replaces GYP's test_installer_sentinel target. BUG=621988 ========== to ========== Add input dependencies on installer test scripts This will cause recompiles when the scripts change, forcing the bots to run the tests. It replaces GYP's test_installer_sentinel target. BUG=621988 Committed: https://crrev.com/ca63fc255102c88323389f36b47d24e7063c4c15 Cr-Commit-Position: refs/heads/master@{#401426} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ca63fc255102c88323389f36b47d24e7063c4c15 Cr-Commit-Position: refs/heads/master@{#401426} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
