|
|
Chromium Code Reviews|
Created:
4 years, 6 months ago by Dirk Pranke Modified:
4 years, 6 months ago CC:
chromium-apps-reviews_chromium.org, chromium-reviews, darin-cc_chromium.org, extensions-reviews_chromium.org, jam Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDisable remaining tests that fail on the Linux desktop CrOS GN builds.
This CL disables five tests that currently fail on the builders, so that
we can flip the builders from GYP to GN without waiting for them to be
fixed.
R=stevenjb@chromium.org, brettw@chromium.org, dcastagna@chromium.org
BUG=618516, 619765, 619784
Committed: https://crrev.com/98afc7a88ebf27d0b5e765ab112fbc6778cd3f05
Cr-Commit-Position: refs/heads/master@{#400235}
Patch Set 1 : more tweaking #
Total comments: 4
Patch Set 2 : only disable in ozone #
Total comments: 2
Patch Set 3 : fix compile failure, disable tests instead of commenting them out #
Dependent Patchsets: Messages
Total messages: 27 (10 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org BUG=618517, 619765, 619784 ========== to ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org BUG=618516, 619765, 619784 ==========
Description was changed from ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org BUG=618516, 619765, 619784 ========== to ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org, dcastagna@chromium.org BUG=618516, 619765, 619784 ==========
dpranke@chromium.org changed reviewers: + dcastagna@chromium.org
See https://codereview.chromium.org/1931033002/#ps360001 for some of the failure logs.
https://codereview.chromium.org/2071703002/diff/40001/chrome/browser/extensio... File chrome/browser/extensions/extension_commands_global_registry_apitest.cc (right): https://codereview.chromium.org/2071703002/diff/40001/chrome/browser/extensio... chrome/browser/extensions/extension_commands_global_registry_apitest.cc:125: #define MAYBE_GlobalCommand DISABLED_GlobalCommand This only fails if defined(USE_OZONE), so maybe only disable it then? https://codereview.chromium.org/2071703002/diff/40001/content/test/ppapi/ppap... File content/test/ppapi/ppapi_browsertest.cc (right): https://codereview.chromium.org/2071703002/diff/40001/content/test/ppapi/ppap... content/test/ppapi/ppapi_browsertest.cc:41: #define MAYBE_BrowserFont DISABLED_BrowserFont These only fail if defined(USE_OZONE), so maybe only disable them then?
https://codereview.chromium.org/2071703002/diff/40001/chrome/browser/extensio... File chrome/browser/extensions/extension_commands_global_registry_apitest.cc (right): https://codereview.chromium.org/2071703002/diff/40001/chrome/browser/extensio... chrome/browser/extensions/extension_commands_global_registry_apitest.cc:125: #define MAYBE_GlobalCommand DISABLED_GlobalCommand On 2016/06/16 00:32:41, stevenjb wrote: > This only fails if defined(USE_OZONE), so maybe only disable it then? ok https://codereview.chromium.org/2071703002/diff/40001/content/test/ppapi/ppap... File content/test/ppapi/ppapi_browsertest.cc (right): https://codereview.chromium.org/2071703002/diff/40001/content/test/ppapi/ppap... content/test/ppapi/ppapi_browsertest.cc:41: #define MAYBE_BrowserFont DISABLED_BrowserFont On 2016/06/16 00:32:41, stevenjb wrote: > These only fail if defined(USE_OZONE), so maybe only disable them then? ok.
Updated, please take another look?
lgtm
https://codereview.chromium.org/2071703002/diff/60001/ui/gl/gl_image_ozone_na... File ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc (right): https://codereview.chromium.org/2071703002/diff/60001/ui/gl/gl_image_ozone_na... ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc:133: // TODO(crbug.com/618516) - The tests in this file can only be run Do they build? If that's the case, can we just add the DISABLED prefix instead of commenting them out? Thank you!
https://codereview.chromium.org/2071703002/diff/60001/ui/gl/gl_image_ozone_na... File ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc (right): https://codereview.chromium.org/2071703002/diff/60001/ui/gl/gl_image_ozone_na... ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc:133: // TODO(crbug.com/618516) - The tests in this file can only be run On 2016/06/16 01:34:57, Daniele Castagna wrote: > Do they build? If that's the case, can we just add the DISABLED prefix instead > of commenting them out? > > Thank you! They do build OK. I didn't think there was a way to do disable these with this macro but I searched again and found an example. It would look like this: INSTANTIATE_TYPED_TEST_CASE_P(DISABLED_GLImageOzoneNativePixmapDrm, ... ) Let's do that instead.
On 2016/06/16 16:42:11, stevenjb wrote: > https://codereview.chromium.org/2071703002/diff/60001/ui/gl/gl_image_ozone_na... > File ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc (right): > > https://codereview.chromium.org/2071703002/diff/60001/ui/gl/gl_image_ozone_na... > ui/gl/gl_image_ozone_native_pixmap_drm_unittest.cc:133: // > TODO(crbug.com/618516) - The tests in this file can only be run > On 2016/06/16 01:34:57, Daniele Castagna wrote: > > Do they build? If that's the case, can we just add the DISABLED prefix instead > > of commenting them out? > > > > Thank you! > > They do build OK. I didn't think there was a way to do disable these with this > macro but I searched again and found an example. It would look like this: > > INSTANTIATE_TYPED_TEST_CASE_P(DISABLED_GLImageOzoneNativePixmapDrm, ... ) > > Let's do that instead. Ack.
lgtm
lgtm ++
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2071703002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
lgtm
The CQ bit was checked by dpranke@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2071703002/80001
Message was sent while issue was closed.
Description was changed from ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org, dcastagna@chromium.org BUG=618516, 619765, 619784 ========== to ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org, dcastagna@chromium.org BUG=618516, 619765, 619784 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org, dcastagna@chromium.org BUG=618516, 619765, 619784 ========== to ========== Disable remaining tests that fail on the Linux desktop CrOS GN builds. This CL disables five tests that currently fail on the builders, so that we can flip the builders from GYP to GN without waiting for them to be fixed. R=stevenjb@chromium.org, brettw@chromium.org, dcastagna@chromium.org BUG=618516, 619765, 619784 Committed: https://crrev.com/98afc7a88ebf27d0b5e765ab112fbc6778cd3f05 Cr-Commit-Position: refs/heads/master@{#400235} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/98afc7a88ebf27d0b5e765ab112fbc6778cd3f05 Cr-Commit-Position: refs/heads/master@{#400235} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
