> I think what we should do is confirm that strict warnings as errors is > actually turned on for xpcshell, and if so, where this happens.
Indeed. Can you set a breakpoint in toggleWerror and see what trips it? Gavin On Thu, May 8, 2014 at 7:02 AM, Eddy Bruel <ejpbr...@mozilla.com> wrote: > To be clear, I don't actually know where the werror flag for xpcshell tests > is set. > > What I've observed is: > - If we Cu.import a JSM that requires module X, and then also require > module X from head_dbg.js after that, all is well. > - If we require module X from head_dbg.js first, and then also require > module X from the JSM after that, we get strict errors. > > What I've gathered from reading the code is: > - Each JSContext has a werror flag, which is set to false by default. > - Each JSScript inherits the werror flag from the JSContext in which it > is compiled. > - loadSubScript uses the JSContext of its caller. > - Cu.import loads JSM's in a separate JSContext, which doesn't have the > flag set. > - Our CommonJS loader uses loadSubScript under the hood, and caches > modules. > > The conclusion I drew from that is that apparently, the werror flag is set > for the context in which head_dbg.js is compiled: if the module gets loaded > from that context first, it inherits the werror flag, causing strict > warnings to be turned into errors. Conversely, if the JSM gets to load the > module first, it gets loaded from the JSM's context, which doesn't have the > flag set, and everything is fine. > > This is the best explanation I could come up with based on the behavior I've > observed. It could be that I missed something, and the werror flag isn't > actually set for xpcshell tests, but if that's the case, they are still > coming from *somewhere*, and this only strenghtens my argument that it's > extremely confusing what causes them to be enabled. So my main point > remains, and this is still an issue for us. > > I think what we should do is confirm that strict warnings as errors is > actually turned on for xpcshell, and if so, where this happens. > > CC'ing bholley, because he probably knows where to look. > > > On 08/05/14 03:45, Gavin Sharp wrote: >> >> To elaborate: >> - Bug 524781 is still open >> - I don't see any reference to -werror or -S in runxpcshelltests.py >> >> Gavin >> >> On Wed, May 7, 2014 at 4:48 PM, Gavin Sharp <ga...@gavinsharp.com> wrote: >>>> >>>> When xpcshell tests are run, they flip a bit on the initial JSContext >>>> that's >>>> off by default that tells spidermonkey "make the strict warning messages >>>> into error messages". >>> >>> Do you have a pointer to where this happens? I've never heard of this, >>> and couldn't find it MXRing. >>> >>> Gavin >>> >>> >>> On Wed, May 7, 2014 at 4:39 PM, Fitzgerald, Nick >>> <nfitzger...@mozilla.com> wrote: >>>> >>>> On 5/7/14, 4:21 PM, Gavin Sharp wrote: >>>>> >>>>> What does "get rid of strict warnings as errors for xpcshell tests" >>>>> mean in practice? >>>> >>>> It means that our non-standard spidermonkey "strict mode" (not the >>>> actual >>>> strict mode) console warnings would continue to simply be console >>>> warning >>>> messages rather than console error messages in xpcshell tests. >>>> >>>> >>>>> I don't understand how you're getting into the situation of >>>>> "accidentally turn[ing] on strict warnings as errors". >>>> >>>> Eddy can explain this better than me because he's been deep in these >>>> trenches the last couple weeks, but I'll give it a shot. >>>> >>>> When xpcshell tests are run, they flip a bit on the initial JSContext >>>> that's >>>> off by default that tells spidermonkey "make the strict warning messages >>>> into error messages". Depending on how you load JS code, you might share >>>> the >>>> JSContext or you might not; for example, loadSubScript shares the >>>> JSContext, >>>> while Cu.import doesn't. Eddy has been making changes to the debugger >>>> server >>>> so that it will run in workers so we can debug workers. He has been >>>> replacing Cu.import calls with calls to a module loader that uses >>>> loadSubScript underneath the hood. So now code that used to be evaluated >>>> with this bit flipped off (because it is off by default and it was >>>> getting >>>> its own JSContext) is being evaluated with the bit on (because it is >>>> inheriting the JSContext from the xpcshell test). The result is error >>>> messages which cause devtools tests to fail. >>>> >>>> _______________________________________________ >>>> dev-platform mailing list >>>> dev-platform@lists.mozilla.org >>>> https://lists.mozilla.org/listinfo/dev-platform >> >> _______________________________________________ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform > > > On 08/05/14 03:45, Gavin Sharp wrote: >> >> To elaborate: >> - Bug 524781 is still open >> - I don't see any reference to -werror or -S in runxpcshelltests.py >> >> Gavin >> >> On Wed, May 7, 2014 at 4:48 PM, Gavin Sharp <ga...@gavinsharp.com> wrote: >>>> >>>> When xpcshell tests are run, they flip a bit on the initial JSContext >>>> that's >>>> off by default that tells spidermonkey "make the strict warning messages >>>> into error messages". >>> >>> Do you have a pointer to where this happens? I've never heard of this, >>> and couldn't find it MXRing. >>> >>> Gavin >>> >>> >>> On Wed, May 7, 2014 at 4:39 PM, Fitzgerald, Nick >>> <nfitzger...@mozilla.com> wrote: >>>> >>>> On 5/7/14, 4:21 PM, Gavin Sharp wrote: >>>>> >>>>> What does "get rid of strict warnings as errors for xpcshell tests" >>>>> mean in practice? >>>> >>>> >>>> It means that our non-standard spidermonkey "strict mode" (not the >>>> actual >>>> strict mode) console warnings would continue to simply be console >>>> warning >>>> messages rather than console error messages in xpcshell tests. >>>> >>>> >>>>> I don't understand how you're getting into the situation of >>>>> "accidentally turn[ing] on strict warnings as errors". >>>> >>>> >>>> Eddy can explain this better than me because he's been deep in these >>>> trenches the last couple weeks, but I'll give it a shot. >>>> >>>> When xpcshell tests are run, they flip a bit on the initial JSContext >>>> that's >>>> off by default that tells spidermonkey "make the strict warning messages >>>> into error messages". Depending on how you load JS code, you might share >>>> the >>>> JSContext or you might not; for example, loadSubScript shares the >>>> JSContext, >>>> while Cu.import doesn't. Eddy has been making changes to the debugger >>>> server >>>> so that it will run in workers so we can debug workers. He has been >>>> replacing Cu.import calls with calls to a module loader that uses >>>> loadSubScript underneath the hood. So now code that used to be evaluated >>>> with this bit flipped off (because it is off by default and it was >>>> getting >>>> its own JSContext) is being evaluated with the bit on (because it is >>>> inheriting the JSContext from the xpcshell test). The result is error >>>> messages which cause devtools tests to fail. >>>> >>>> _______________________________________________ >>>> dev-platform mailing list >>>> dev-platform@lists.mozilla.org >>>> https://lists.mozilla.org/listinfo/dev-platform >> >> _______________________________________________ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform > > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
