GTTF: Remove broken slave "Linux Reliability (valgrind and tsan)"

GTTF: Remove broken slave "Linux Reliability (valgrind and tsan)"

BUG=288383
R=thakis@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=225092

[Nico, 2013-09-19 23:15:15]

Was this removal discussed anywhere? The bug "remove unused targets; if they are
used they won't be removed" but this target was apparently used (?)

[Paweł Hajdan Jr., 2013-09-19 23:18:36]

Stefan, could you review?

I've talked to QA about this and these tests are just not used.

[Nico, 2013-09-20 00:14:11]

What do you mean with "not used"? They run on the memory waterfall, which
had a sheriff rotation.
On Sep 19, 2013 4:18 PM, <phajdan.jr@chromium.org> wrote:

> Reviewers: Nico, szager1,
>
> Message:
> Stefan, could you review?
>
> I've talked to QA about this and these tests are just not used.
>
> Description:
> GTTF: Remove broken slave "Linux Reliability (valgrind and tsan)"
>
> BUG=288383
>
> Please review this at
https://codereview.chromium.**org/24299003/<https://codereview.chromium.org/2...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/tools/build/<http://svn.chromium.org/chrome/trunk/tools/build/>
>
> Affected files (+0, -33 lines):
>   M     masters/master.chromium.**memory.fyi/master.cfg
>   M     masters/master.chromium.**memory.fyi/slaves.cfg
>
>
> Index: masters/master.chromium.**memory.fyi/master.cfg
> ==============================**==============================**=======
> --- masters/master.chromium.**memory.fyi/master.cfg       (revision
> 224226)
> +++ masters/master.chromium.**memory.fyi/master.cfg       (working copy)
> @@ -62,7 +62,6 @@
>                         branch='src',
>                         treeStableTimer=60,
>                         builderNames=[# BuildTesters:
> -                                     'Linux Reliability (valgrind and
> tsan)',
>                                       'Webkit Linux (valgrind layout)',
>                                       'Windows Tests (tsan)',
>                                       'Linux Heapcheck',
> @@ -306,21 +305,6 @@
>      tests=['valgrind_unit_3_of_3']**,
>      factory_properties={ 'needs_valgrind' : True, })
>
> -f_chromium_rel_linux_**valgrind_reliability = F_LINUX(
> -    target='Release',
> -    tests=[
> -        'valgrind_reliability',
> -        'tsan_reliability',
> -    ],
> -    options=[
> -        'reliability_tests',
> -    ],
> -    factory_properties={
> -        'needs_valgrind' : True,
> -        'gclient_env': { 'GYP_DEFINES' : 'build_for_tool=memcheck'}
> -    }
> -)
> -
>  # Mac Valgrind bots:
>  # We use debug builds for mac valgrind bots because we can't get stacks on
>  # release builds.
> @@ -850,14 +834,6 @@
>    'auto_reboot': False,
>  }
>
> -b_chromium_rel_linux_**valgrind_reliability = {
> -  'name': 'Linux Reliability (valgrind and tsan)',
> -  'builddir': 'chromium-rel-linux-valgrind-**reliability',
> -  'factory': f_chromium_rel_linux_valgrind_**reliability,
> -  'category': '1Linux Valgrind',
> -  'auto_reboot': False,
> -}
> -
>  b_chromium_rel_mac_valgrind_**builder = {
>    'name': 'Chromium Mac Builder (valgrind)',
>    'builddir': 'chromium-rel-mac-valgrind-**builder',
> @@ -1052,7 +1028,6 @@
>    b_chromium_rel_linux_valgrind_**tests_3,
>    b_chromium_rel_linux_valgrind_**tests_4,
>    b_chromium_rel_linux_valgrind_**tests_5,
> -  b_chromium_rel_linux_valgrind_**reliability,
>
>    b_chromium_rel_mac_valgrind_**builder,
>    b_chromium_dbg_mac_valgrind_1,
> Index: masters/master.chromium.**memory.fyi/slaves.cfg
> ==============================**==============================**=======
> --- masters/master.chromium.**memory.fyi/slaves.cfg       (revision
> 224226)
> +++ masters/master.chromium.**memory.fyi/slaves.cfg       (working copy)
> @@ -107,14 +107,6 @@
>    },
>    {
>      'master': 'ChromiumMemoryFYI',
> -    'hostname': 'vm901-m1',
> -    'builder': 'Linux Reliability (valgrind and tsan)',
> -    'os': 'linux',
> -    'version': 'precise',
> -    'bits': '64',
> -  },
> -  {
> -    'master': 'ChromiumMemoryFYI',
>      'hostname': 'build62-m1',
>      'builder': 'Webkit Linux (valgrind layout)',
>      'os': 'linux',
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Paweł Hajdan Jr., 2013-09-20 00:34:02]

reliability_tests only ran a single test, PageLoadTest.Reliability .

These tests are being "decommissioned" as I've mentioned before. They are
inherently broken by design (automation is just so flaky). QA has acknowledged
the removal.

[Nico, 2013-09-20 03:53:53]

On Thu, Sep 19, 2013 at 5:34 PM, <phajdan.jr@chromium.org> wrote:

> reliability_tests only ran a single test, PageLoadTest.Reliability .
>

Right, but that test does lots of stuff irrc.


> These tests are being "decommissioned" as I've mentioned before.


That's exactly what I'm asking about. Can you point me to where (bug, email
thread, …) the decision to decommission them happened? (I think I vaguely
remember reading that somewhere, but I don't remember where and I'm not
sure if I'm not just making this up.)


> They are inherently broken by design (automation is just so flaky). QA has
> acknowledged
> the removal.
>

That's good, but QA usually doesn't decide which test binaries we run or
don't run :-)


>
>
https://codereview.chromium.**org/24299003/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[navabi, 2013-09-20 21:10:06]

On 2013/09/20 03:53:53, Nico wrote:
> On Thu, Sep 19, 2013 at 5:34 PM, <mailto:phajdan.jr@chromium.org> wrote:
> 
> > reliability_tests only ran a single test, PageLoadTest.Reliability .
> >
> 
> Right, but that test does lots of stuff irrc.
> 
> 
> > These tests are being "decommissioned" as I've mentioned before.
> 
> 
> That's exactly what I'm asking about. Can you point me to where (bug, email
> thread, …) the decision to decommission them happened? (I think I vaguely
> remember reading that somewhere, but I don't remember where and I'm not
> sure if I'm not just making this up.)
> 
> 
> > They are inherently broken by design (automation is just so flaky). QA has
> > acknowledged
> > the removal.
> >
> 
> That's good, but QA usually doesn't decide which test binaries we run or
> don't run :-)
> 
> 
> >
> >
>
https://codereview.chromium.**org/24299003/%3Chttps://codereview.chromium.org...>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Drive by review: I don't know enough about these tests to know if they should be
removed or not.
There should be a person or some people that we can add to this review to
determine if anyone is looking at these (besides just sheriffs to see if they
are green).
If no one knows that person (or those people), let's send an email out to the
team asking people to speak up if they want to keep it or else we remove it?
WDYT?

[Paweł Hajdan Jr., 2013-09-20 23:04:43]

+Timur who added the bot

This is now the _only_ bot that tries to run this test. For anyone who knows
anything about reliability tests, this makes no sense. QA has migrated their
reliability tests to something different and is not using this any more.

Please let me know if you have any question, but I'd just like to remove no
longer used tests.

[Timur Iskhodzhanov, 2013-09-22 06:41:08]

Let me put it this way: reliability_tests WAS a target that QAs used to test
reliability and it was indeed abandoned some time ago.

However, long before it was abandoned, we've started using it for
Valgind/TSan/etc testing and it has proved to be very useful as it has found
numerous real bugs only observable on the full browser run. We can also easily
control the time it takes to run the test by adding/removing URLs to the list.

That said, we'd very much appreciate if the removal was reverted.
We only need the PageLoadTest.Reliability to cycle through a list of URLs.
I'm fine if it's renamed to url_cycle_test or something like that.

[Timur Iskhodzhanov, 2013-09-22 06:42:13]

re: flakiness - yes we know automation is flaky, but we almost entirely don't
use it in this test.
Running a large program under dynamic testing tools is flaky anyways :)

[Timur Iskhodzhanov, 2013-09-23 16:13:25]

cc: current MFYI sheriff, just FYI

[Timur Iskhodzhanov, 2013-09-24 16:39:53]

ping?


2013/9/21 <timurrrr@chromium.org>

> Let me put it this way: reliability_tests WAS a target that QAs used to
> test
> reliability and it was indeed abandoned some time ago.
>
> However, long before it was abandoned, we've started using it for
> Valgind/TSan/etc testing and it has proved to be very useful as it has
> found
> numerous real bugs only observable on the full browser run. We can also
> easily
> control the time it takes to run the test by adding/removing URLs to the
> list.
>
> That said, we'd very much appreciate if the removal was reverted.
> We only need the PageLoadTest.Reliability to cycle through a list of URLs.
> I'm fine if it's renamed to url_cycle_test or something like that.
>
>
https://codereview.chromium.**org/24299003/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Timur Iskhodzhanov, 2013-09-24 18:29:16]

I've chatted with Pawel and we couldn't come up with a consensus yet, so
I've filed a bug to track progress
https://code.google.com/p/chromium/issues/detail?id=297759

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jam, 2013-09-24 19:42:27]

On 2013/09/22 06:41:08, Timur Iskhodzhanov wrote:
> Let me put it this way: reliability_tests WAS a target that QAs used to test
> reliability and it was indeed abandoned some time ago.
> 
> However, long before it was abandoned, we've started using it for
> Valgind/TSan/etc testing and it has proved to be very useful as it has found
> numerous real bugs only observable on the full browser run. We can also easily
> control the time it takes to run the test by adding/removing URLs to the list.
> 
> That said, we'd very much appreciate if the removal was reverted.
> We only need the PageLoadTest.Reliability to cycle through a list of URLs.
> I'm fine if it's renamed to url_cycle_test or something like that.

hey Timur, I'm sympathetic that this test that you used is now gone. However
automation has been deprecated over a year and the plans to remove it have been
public. If there are particular actions that this test target did which caught
leaks that other tests didn't, can you create new browser tests or telemetry
based tests to accomplish this?

[Nico, 2013-09-24 19:50:01]

lgtm based on jam pointing out that this is part of http://crbug.com/224072

I think this CL would have moved faster had this been pointed out earlier :-)

[Nico, 2013-09-24 20:05:51]

On Tue, Sep 24, 2013 at 12:50 PM, <thakis@chromium.org> wrote:

> lgtm based on jam pointing out that this is part of
> http://crbug.com/224072


(Namely, here:
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/6lcguaMmjqA/dis...)


>
>
> I think this CL would have moved faster had this been pointed out earlier
> :-)
>
>
https://codereview.chromium.**org/24299003/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[navabi, 2013-09-24 22:26:46]

On 2013/09/24 20:05:51, Nico wrote:
> On Tue, Sep 24, 2013 at 12:50 PM, <mailto:thakis@chromium.org> wrote:
> 
> > lgtm based on jam pointing out that this is part of
> > http://crbug.com/224072
> 
> 
> (Namely, here:
>
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/6lcguaMmjqA/dis...)
> 
> 
> >
> >
> > I think this CL would have moved faster had this been pointed out earlier
> > :-)
> >
> >
>
https://codereview.chromium.**org/24299003/%3Chttps://codereview.chromium.org...>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

lgtm

[Paweł Hajdan Jr., 2013-09-24 22:41:12]

Committed patchset #1 manually as r225092 (presubmit successful).