675283 - Run Tracemonkey tests on mozilla-inbound for changes involving js/src

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[(no longer active)]

The js team needs some special kinds of tests to be run on js/src changes (as they are on the Tracemonkey branch now).  I'm filing this bug to get those tests on mozilla-inbound, to allow the js team to completely switch to mozilla-inbound.

catlee told me that this is possible.  I'm sure he'll add a comment about how it should be done.

Comment 1

[Chris AtLee [:catlee]]

I think we just need to change the spidermonkey project configs to look at mozilla-inbound instead of tracemonkey.

Comment 2

[Phil Ringnalda (:philor)]

I'd include check-valgrind in the list of "some special kinds of tests."

Comment 3

[Aki Sasaki (not active)]

(In reply to comment #0)
> to allow the js team to completely switch to
> mozilla-inbound.

Does that mean the Tracemonkey branch is going away?

Comment 4

[Aki Sasaki (not active)]

Attachment: tracemonkey tests on inbound

If we're getting rid of Tracemonkey, this is probably right.
If we're not, this may be a bit too much.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 549976 [details] [diff] [review]
tracemonkey tests on inbound

Review of attachment 549976 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozilla/config.py
@@ +893,5 @@
>              'macosx': PLATFORM_VARS['macosx']['env'],
>              'macosx-debug': PLATFORM_VARS['macosx-debug']['env'],
>          },
>          'hgurl': 'http://hg.mozilla.org',
> +        'repo_path': '/integration/mozilla-inbound',

Drop the leading slash here, we don't use it elsewhere (and I'm not sure whether it will bust something or not...).

Note that this also stops running them on TraceMonkey, which will be a problem if the switch-to-inbound plan is aborted.

::: mozilla/project_branches.py
@@ +99,5 @@
> +            'linux-debug': {
> +                'enable_valgrind_checktests': True,
> +            },
> +            'linux64-debug': {
> +                'enable_valgrind_checktests': True,

Can we get a separate bug on file for this? These are currently run _only_ on TraceMonkey, and it seems like we ought to be running them everywhere instead of just on inbound -- I don't have the insight to say for sure, though.

@@ +112,5 @@
>              'remote-tp4m': 1,
>              'remote-tp4m_nochrome': 1,
>              'remote-twinopen': 1,
>              'remote-tzoom': 1,
> +            'v8': 1,

Same thing for this.

Comment 6

[(no longer active)]

The goal here is to get rid of the TraceMonkey branch, yes.

Comment 7

[Paul Biggar]

(In reply to comment #5)
> ::: mozilla/project_branches.py
> @@ +99,5 @@
> > +            'linux-debug': {
> > +                'enable_valgrind_checktests': True,
> > +            },
> > +            'linux64-debug': {
> > +                'enable_valgrind_checktests': True,
> 
> Can we get a separate bug on file for this? These are currently run _only_
> on TraceMonkey, and it seems like we ought to be running them everywhere
> instead of just on inbound -- I don't have the insight to say for sure,
> though.

Does this just call "make check-valgrind"? If so, it would probably be easier to do that in js/src/Makefile.in as part of "make check".

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 549976 [details] [diff] [review]
tracemonkey tests on inbound

r+ for the config.py part and the build_space changes. I'd like to see some discussion and/or sign off on what to do about the valgrind check tests and v8 stuff, though (put them everywhere vs. only on inbound vs. only on m-c/inbound etc.). Probably should happen in a separate bug.

Comment 9

[Aki Sasaki (not active)]

(In reply to comment #8)
> Comment on attachment 549976 [details] [diff] [review] [diff] [details] [review]
> tracemonkey tests on inbound
> 
> r+ for the config.py part and the build_space changes. I'd like to see some
> discussion and/or sign off on what to do about the valgrind check tests and
> v8 stuff, though (put them everywhere vs. only on inbound vs. only on
> m-c/inbound etc.). Probably should happen in a separate bug.

Is this acceptable to the developers on this bug?

Comment 10

[Aki Sasaki (not active)]

Attachment: as requested

(In reply to comment #8)
> Comment on attachment 549976 [details] [diff] [review] [diff] [details] [review]
> tracemonkey tests on inbound
> 
> r+ for the config.py part and the build_space changes. I'd like to see some
> discussion and/or sign off on what to do about the valgrind check tests and
> v8 stuff, though (put them everywhere vs. only on inbound vs. only on
> m-c/inbound etc.). Probably should happen in a separate bug.

This is the patch with only the config.py part and build_space changes.

Marking as r+ and leaving it up to the TM folks to start the v8/valgrind discussion and/or bug.

Comment 11

[Aki Sasaki (not active)]

Comment on attachment 551498 [details] [diff] [review]
as requested

http://hg.mozilla.org/build/buildbot-configs/rev/8d5e89e9c10e

Comment 12

[Luke Wagner [:luke]]

(In reply to Aki Sasaki [:aki] from comment #10)
> Marking as r+ and leaving it up to the TM folks to start the v8/valgrind
> discussion and/or bug.

I don't have any strong opinions; Paul, do you?

Comment 13

[Paul Biggar]

> (In reply to Aki Sasaki [:aki] from comment #10)
> > Marking as r+ and leaving it up to the TM folks to start the v8/valgrind
> > discussion and/or bug.

What does v8 refer to here? I wasn't aware of anything in our test suite that involved v8.

I'm not sure what to do about valgrind because I don't fully understand when |make check-valgrind| is run. Can you give me more details about this? (I'm trying to determine whether just adding a Makefile rule like "check:: check-valgrind" will solve this.)

Comment 14

[Aki Sasaki (not active)]

(In reply to Paul Biggar from comment #13)
> > (In reply to Aki Sasaki [:aki] from comment #10)
> > > Marking as r+ and leaving it up to the TM folks to start the v8/valgrind
> > > discussion and/or bug.
> 
> What does v8 refer to here? I wasn't aware of anything in our test suite
> that involved v8.

I don't know, but a Talos suite involving v8 was running on Tracemonkey. http://is.gd/eLcgAZ shows the WinNT 6.1 graph.

> I'm not sure what to do about valgrind because I don't fully understand when
> |make check-valgrind| is run. Can you give me more details about this? (I'm
> trying to determine whether just adding a Makefile rule like "check::
> check-valgrind" will solve this.)

It looks like a 'make check-valgrind' inside of js/src, which we don't do anywhere else.

If 'make check' in the top of objdir were changed to also run 'make check-valgrind' in js/src, that would be the same thing, but would run in other branches as well.

I'm not sure what this means in terms of overall build time; if 'make check-valgrind' takes a long time, we may not want to do it on every build.

Comment 15

[Paul Biggar]

(In reply to Aki Sasaki [:aki] from comment #14)
> (In reply to Paul Biggar from comment #13)
> > > (In reply to Aki Sasaki [:aki] from comment #10)
> > > > Marking as r+ and leaving it up to the TM folks to start the v8/valgrind
> > > > discussion and/or bug.
> > 
> > What does v8 refer to here? I wasn't aware of anything in our test suite
> > that involved v8.
>
> I don't know, but a Talos suite involving v8 was running on Tracemonkey.
> http://is.gd/eLcgAZ shows the WinNT 6.1 graph.

Ah, and this ran on no other trees? I guess we want this then.


 
> > I'm not sure what to do about valgrind because I don't fully understand when
> > |make check-valgrind| is run. Can you give me more details about this? (I'm
> > trying to determine whether just adding a Makefile rule like "check::
> > check-valgrind" will solve this.)
> 
> It looks like a 'make check-valgrind' inside of js/src, which we don't do
> anywhere else.


I meant in terms of the infratructure, when is it run? Is valgrind installed on all machines? Was |make check-valgrind| run on every push on TM, or selectively depending on the build machine selected, or something else?

> I'm not sure what this means in terms of overall build time; if 'make
> check-valgrind' takes a long time, we may not want to do it on every build.

I think only a very small number of JS tests use valgrind, and the build time barely increases. I'm thinking we should just run it unconditionally.

Comment 16

[Nicholas Nethercote [inactive]]

> I think only a very small number of JS tests use valgrind,

Yes, it's only 3, IIRC>

> and the build
> time barely increases. I'm thinking we should just run it unconditionally.

Yes please!

Comment 17

[Paul Biggar]

(In reply to Nicholas Nethercote [:njn] from comment #16)
> > and the build
> > time barely increases. I'm thinking we should just run it unconditionally.
> 
> Yes please!

I measured this. It adds 30s to a debug build on Mac.

Comment 18

[Aki Sasaki (not active)]

(In reply to Paul Biggar from comment #15)
> (In reply to Aki Sasaki [:aki] from comment #14)
> > (In reply to Paul Biggar from comment #13)
> > > > (In reply to Aki Sasaki [:aki] from comment #10)
> > > > > Marking as r+ and leaving it up to the TM folks to start the v8/valgrind
> > > > > discussion and/or bug.
> > > 
> > > What does v8 refer to here? I wasn't aware of anything in our test suite
> > > that involved v8.
> >
> > I don't know, but a Talos suite involving v8 was running on Tracemonkey.
> > http://is.gd/eLcgAZ shows the WinNT 6.1 graph.
> 
> Ah, and this ran on no other trees? I guess we want this then.

Ok. Per comment 5:

(In reply to Ben Hearsum [:bhearsum] from comment #5)
> Can we get a separate bug on file for this? These are currently run _only_
> on TraceMonkey, and it seems like we ought to be running them everywhere
> instead of just on inbound -- I don't have the insight to say for sure,
> though.

This may be asking for more information than you currently have as well.
Do we need more discussion, or is just-inbound or all-branches an easy decision to make here?


(In reply to Paul Biggar from comment #15)
> I meant in terms of the infratructure, when is it run? Is valgrind installed
> on all machines? Was |make check-valgrind| run on every push on TM, or
> selectively depending on the build machine selected, or something else?

It looks like |make check-valgrind| was run on every push on TM.
Pretty sure we don't selectively install valgrind; our build slaves [of a particular platform] should be identical.

> I think only a very small number of JS tests use valgrind, and the build
> time barely increases. I'm thinking we should just run it unconditionally.

Ok.
Does that mean adding |make check-valgrind| as a |make check| makefile dependency and removing the conditional enable_valgrind_checktests on the buildbot configs side?

Comment 19

[Paul Biggar]

(In reply to Aki Sasaki [:aki] from comment #18)
> Ok. Per comment 5:
> 
> (In reply to Ben Hearsum [:bhearsum] from comment #5)
> > Can we get a separate bug on file for this? These are currently run _only_
> > on TraceMonkey, and it seems like we ought to be running them everywhere
> > instead of just on inbound -- I don't have the insight to say for sure,
> > though.
> 
> This may be asking for more information than you currently have as well.
> Do we need more discussion, or is just-inbound or all-branches an easy
> decision to make here?

If this ran only on TM, then we only need it on mozilla-inbound. It may be nice to have it elsewhere (and feel free to do that), but I wouldn't feel compelled t do it everywhere unless someone speaks up.



> (In reply to Paul Biggar from comment #15)
> Does that mean adding |make check-valgrind| as a |make check| makefile
> dependency and removing the conditional enable_valgrind_checktests on the
> buildbot configs side?

I'll make the change in js/src/Makefile.in, and you can remove the buildbot configs. check-valgrind won't exist anymore. I'll file a bug in a moment.

Comment 20

[Aki Sasaki (not active)]

Attachment: v8 on inbound only

per comment 19.

Using hg diff -U20 for enough context =P

Comment 21

[Aki Sasaki (not active)]

Comment on attachment 551664 [details] [diff] [review]
v8 on inbound only

http://hg.mozilla.org/build/buildbot-configs/rev/08848f47add8

Comment 22

[Aki Sasaki (not active)]

Attachment: spidermonkey.sh : tracemonkey -> mozilla-inbound

Noticed that there are a bunch of spidermonkey tests running red on the Tracemonkey tinderbox page (hidden).

http://tinderbox.mozilla.org/showbuilds.cgi?tree=TraceMonkey&noignore=1

This patch gets them pulling from mozilla-inbound instead of tracemonkey.
Followup patch coming to send them to the mozilla-inbound tree.

Comment 23

[Aki Sasaki (not active)]

Attachment: send spidermonkey results to the mozilla-inbound tinderbox page

Comment 24

[Aki Sasaki (not active)]

Comment on attachment 551828 [details] [diff] [review]
send spidermonkey results to the mozilla-inbound tinderbox page

http://hg.mozilla.org/build/buildbot-configs/rev/92012edde182

Comment 25

[Aki Sasaki (not active)]

With the build/tools checkin, we got an orange on mozilla-inbound_linux-debug_spidermonkey-notracejit rather than a red.

It'll appear on the mozilla-inbound page after a reconfig.

Comment 26

[Armen [:armenzg]]

We're only running the v8 test suite on mozilla-inbound. Is this still the expected coverage?

My question in other words: "is this a suite that should close the Firefox tree?".
If the answer is "no" then we can leave it as is.
If it is, then we should enable it in most places.

Comment 27

[Armen [:armenzg]]

What is the reason that we run "v8" only on inbound?

Comment 28

[Justin Lebar (not reading bugmail)]

(In reply to Armen Zambrano G. [:armenzg] - Release Engineer from comment #27)
> What is the reason that we run "v8" only on inbound?

bug 733490 asks the same question!