Run `make buildall` in chromeos-ec src_compile() |
|||||
Issue descriptionHave seen 3 failed paladin runs with errors in ec unittest due to 2 different bad CLs. There's no unittest stage for ec pre-cq set. Should the ec pre-cq set contain at least one pre-cq with unittest stage? https://uberchromegw.corp.google.com/i/chromeos/builders/nyan-paladin/builds/10451/steps/UnitTest/logs/stdio https://uberchromegw.corp.google.com/i/chromeos/builders/nyan-paladin/builds/10452/steps/UnitTest/logs/stdio https://uberchromegw.corp.google.com/i/chromeos/builders/nyan-paladin/builds/10479/steps/UnitTest/logs/stdio
,
Aug 30 2016
https://chromium-review.googlesource.com/#/c/373819/ https://chromium-review.googlesource.com/#/c/373818/ caused failure in https://uberchromegw.corp.google.com/i/chromeos/builders/nyan-paladin/builds/10479/steps/UnitTest/logs/stdio https://chromium-review.googlesource.com/#/c/365448/ caused failure in https://uberchromegw.corp.google.com/i/chromeos/builders/nyan-paladin/builds/10451/steps/UnitTest/logs/stdio https://uberchromegw.corp.google.com/i/chromeos/builders/nyan-paladin/builds/10452/steps/UnitTest/logs/stdio I see compile-only-pre-cq.
,
Aug 30 2016
,
Aug 30 2016
,
Aug 30 2016
compile-only should be running "make -j buildall" as part of test for the chromeos-ec package. Is this not the case?
,
Aug 30 2016
I don't know, but in the meantime while this is diagnosed we should restore the prior ec testing behavior.
,
Aug 30 2016
Actually, it looks like it just builds for that board and not buildall. My bad, we can land the revert while we find a way to do this properly.
,
Aug 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/12b958dd8ad1e8d75308f924a0794448439d9b31 commit 12b958dd8ad1e8d75308f924a0794448439d9b31 Author: Aviv Keshet <akeshet@chromium.org> Date: Tue Aug 30 22:05:55 2016 Revert "COMMIT-QUEUE.ini: Change builders to compile-only." This reverts commit 654271b86c971d7ae32b3f8d798bd1c2c7171f4e. Apparently ec changes can and do break unit tests on the cq. BUG= chromium:642503 TEST=None Change-Id: I88cebf492a4c4a86b75a27ba823a03ffb705a2bb Reviewed-on: https://chromium-review.googlesource.com/378415 Tested-by: Aviv Keshet <akeshet@chromium.org> Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/12b958dd8ad1e8d75308f924a0794448439d9b31/COMMIT-QUEUE.ini
,
Aug 30 2016
I'll check to see if there's a builder config that will run the unittests or make one if necessary.
,
Aug 30 2016
What we probably should do is have one ebuild that builds all the ToT images for all BOARDs supported in the EC firmware repo. Just running "make buildall" does that, and runs the unit tests too. Then we'd only need to do that once on a trybot and CQ. The firmware shellball found in chromiumos_image.bin gets its firmware blobs from BCS. They're only put up there and marked valid after they've run through FAFT and other tests. Any EC firmware blobs that are being built as part of a specific target disk image aren't being used (which was Aseda's point), so there's no need to be doing it on every builder.
,
Aug 30 2016
One other thing that I was thinking about was just having the src_compile() function just call "make buildall", which might be simpler.
,
Aug 30 2016
We should chat. I had a brief email exchange with Bernie about this the other week.
,
Aug 30 2016
Alright, sounds good.
,
Sep 6 2016
After discussing with wfrichar@ last week, long term we'd like to make a separate package that just builds all the EC images for all ToT boards. However in the meantime, we can still gain some benefit by just adding `make buildall` to the compile stage and use the compile-only-pre-cq builders again.
,
Sep 6 2016
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/d33a2d6abef3a73756416d4a87aaaa30ddafae98 commit d33a2d6abef3a73756416d4a87aaaa30ddafae98 Author: Aseda Aboagye <aaboagye@google.com> Date: Tue Sep 06 17:11:48 2016 chromeos-ec: Remove extra make command. `make buildall` will also build the runtests target, so it's redundant here. BUG= chromium:642503 BRANCH=None TEST=emerge chromeos-ec Change-Id: Id48fe28f4573bf52d22299d8d6de3ec83f37a461 Signed-off-by: Aseda Aboagye <aaboagye@google.com> Reviewed-on: https://chromium-review.googlesource.com/381323 Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Reviewed-by: Bill Richardson <wfrichar@chromium.org> [modify] https://crrev.com/d33a2d6abef3a73756416d4a87aaaa30ddafae98/chromeos-base/chromeos-ec/chromeos-ec-9999.ebuild
,
Sep 8 2016
For posterity, we decided against adding `make buildall` to the compile stage and left it in the test stage. However, we are changing the pre-cq builders to no-vmtest builders. That way we'll still run the UnitTest stage, but not the VMTest or HWTest.
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/64d6f5781becdf43b8de29a242277bc98b3b4b1e commit 64d6f5781becdf43b8de29a242277bc98b3b4b1e Author: Aseda Aboagye <aaboagye@google.com> Date: Tue Sep 06 17:06:03 2016 COMMIT-QUEUE.ini: Change builders to no-vmtest-pre-cq. (Re-attempt of https://chromium-review.googlesource.com/#/c/372325/) Really, we only care if the chromeos-ec package unit tests fail and not if the VMTest or ImageTest stages fail. Those test stages don't actually test aganist our EC changes anways, so it's kind of a waste of time to run them. Besides, that's what FAFT is for. BUG= chromium:642503 BRANCH=None TEST=cbuildbot --remote chell-no-vmtest-pre-cq Change-Id: I1b4b7fc68a9f8a943f6f5ef3d8b169264c95359e Signed-off-by: Aseda Aboagye <aaboagye@google.com> Reviewed-on: https://chromium-review.googlesource.com/381106 Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Reviewed-by: Bernie Thompson <bhthompson@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/64d6f5781becdf43b8de29a242277bc98b3b4b1e/COMMIT-QUEUE.ini
,
Sep 9 2016
So, I don't think the change in c#18 is working correctly??? It seems that commit landed at 15:36 PDT. Here's an EC change that was uploaded after that (https://chromium-review.googlesource.com/#/c/382662/). However, the pre-cq builders selected are mixed pre-cq & compile-only... What's going on here? Why weren't the no-vmtest-pre-cq builders chosen?
,
Sep 9 2016
Wow, I think I figured it out. I had a mistake in the config, but someone with fresher eyes had to point it out to me. I uploaded another CL to fix this. https://chromium-review.googlesource.com/#/c/383878/ Why didn't cbuildbot complain??
,
Sep 12 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by akes...@chromium.org
, Aug 30 2016