mozilla :: #ateam

11 Jul 2017
12:00ekylemarco: whenever you are ready
12:00marcoI'm coming
12:00marcoekyle: ^
12:00ekylemarco: greg is still not around
12:47marcogmierz|afk: here's an example Elm app: https://github.com/mozilla-releng/services/blob/master/src/shipit_frontend/src/App/CodeCoverage.elm
12:47marcohttp://elm-lang.org/examples
12:56gmierz|afkmarco: Thanks! It doesn't look that bad tbh (but I've used pascal before).
12:56marcogmierz|afk: no, it's a really good language
12:57marcogmierz: its main benefit is that there are no runtime exceptions, so the code always works once it's compiled
13:00gmierzmarco: Ah, that's always nice! Definitely something I should look into then.
13:00marcogmierz: it would definitely make the app more reliable and easier to maintain
13:04gmierzmarco: Cool, do you know how to write in elm?
13:05marcogmierz: I'm not an expert, but I know a few things
13:05marcothere was a elm session during the all hands
13:06gmierzOh ok, still more than me :) Do you have a link to the page which runs the elm app you gave me an example of?
13:09marcogmierz: the server side is currently broken, so it is not rendering anything, I will send you the link shortly
13:09marcogmierz: here's a smaller example https://ellie-app.com/3zx9gkpfR2Ha1/9
13:16gmierzmarco: Thanks, I'm kind of leaning towards trying it out now...there's enough information out there to help me also.
13:17marcogmierz: if you have doubts you can ask garbas in #shipit, he has a lot of experience with Elm
13:17gmierzmarco: Ok sounds good
13:29gmierzmarco: You mentioned that the server-side is broken. If I write the app in elm, I imagine it'll also run on that server, so I'm wondering if this happens often? And does the server being down also affect the DB that will hold the diff information (i.e. would it affect a js app also)?
13:42marcogmierz: no it almost never happens, it's broken because of a bug in my code :P
13:44gmierzmarco: Haha ok cool
13:45marcogmierz: a really simple one https://github.com/mozilla-releng/services/pull/490/files
13:50gmierzAh that's not bad. With elm, I'd be sending "GET" requests to get the data from you, is that right?
13:58marcogmierz: yes
13:58gmierzty
15:05ekylegmierz: marco I am summarizing our call here: https://public.etherpad-mozilla.org/p/code_coverage_Q3_17 please insert corrections
15:12ekylegmierz: marco look it over, not just for correctness, but from RelMan perspective; does this do what they want
15:20gmierzekyle: marco: Thanks! Do you think we could return the source lines in the request also? It'll save us from having to parse the diff twice, once on the server, and once on the client to get the lines (without the headers and split by source file).
15:22gmierzBy source lines, I mean the content of the line in the files.
15:25ekylegmierz: marco: I will leave the discussion of what actually is sent between the client and the codecoverage server to you both.
15:26gmierzekyle: Sounds good
15:31Standard8ahal: so currently the package lists are similar between the top-level eslint & the plugin one though looking at the top-level, were including a lot that shouldnt be there.
15:33Standard8ahal: so if Im understanding you comments correctly, wed have the node_modules downloaded onto the image, and wed just need to move them into place?
15:34ahalStandard8: ah right, because the checkout doesn't exist yet.. I guess that's why eslint was setup that way
15:34ahalbut yeah, we could create a link, or move them
15:34ahalStandard8: if you want I can take a stab at a patch for you
15:35Standard8ahal: so how about we just use the lint image, and download both of them onto it?
15:35Standard8ahal: sure
15:35Standard8I can go look at my reviews ;-)
15:35ahalStandard8: sure that would work too
15:35ahalI just thought if the eslint one was basically a superset of the plugin one, then the plugin one wouldn't be needed
15:36ahalI mean, the plugin dependencies are installed somehow for the ES task right?
15:36ahalor is that at runtime
15:36ahaler, s/runtime/during mach lint
15:36Standard8ahal: yeah, the dependencies should all be there, its just the additional mocha dev dependency
15:39ahalStandard8: ah.. ok. sorry I thought all that was for eslint-plugin-mozilla
15:39ahalyeah, makes sense to keep that separate from the eslint one then
15:45Standard8ahal: so just to be clear, are you doing a patch for that, or shall I?
15:46ahalStandard8: I'll do one for the eslint tooltool archive and file it in a separate bug, then you can copy that
15:46ahaljust want to make sure I'm not telling you to do something that won't work for some reason
15:46Standard8ahal: sure
15:52marcoekyle: gmierz: this is the bug about disabling optimizations in ccov builds: https://bugzilla.mozilla.org/show_bug.cgi?id=1344165
15:52bugbotBug 1344165: Code Coverage, normal, nobody, NEW , Disable optimizations in the linux64-ccov build
15:55b4handato: hey thanks for the reply. It turned out to be the profile. I passed in a profile that was blank but had the first run already done and it works. I don't know exactly what the profile needs to have different though
15:56marcoekyle: can you give me an activedata query to get lines covered in a given file?
15:58ekylemarco: ok....
16:03ekylemarco: https://activedata.allizom.org/tools/query.html#query_id=BhX4e9g2
16:04marcoekyle: thanks!
16:04ekylemarco: that will cover all tests that touch that file, for that revision
16:05ekylemarco: and feel free to ask more if you have other questions.
16:07gmierzmarco: Cool, looks like we've got a lot of timeouts haha. Have you looked into them anymore after that last comment on the bug?
16:08marcogmierz: not yet, I forgot :P
16:08marcoekyle: one more question, regarding the results of the query, why is there info like "name":"test-linux64-ccov/opt-web-platform-tests-2" ?
16:08gmierzmarco: All good, I think we can solve a few of those by increasing the test-suite timeouts
16:09ekylemarco: that was the test suite chunk that the coverage came from
16:09marcoekyle: the coverage is only for that test or is it the sum of coverage from all tests?
16:10ekylemarco: only for that chunk, that set of tests run in that chunk
16:10ekylemarco: so, there can be other chunks that cover this file. ActiveData does not merge the coverage artifacts
16:11marcoekyle: okok, so, if I wanted to get the overall coverage
16:11ekylemarco: there is refined data in "coverage-summary" with all tests, all chunks merged
16:12marcoekyle: "from": "coverage-summary"?
16:12ekylemarco: ..but it appears that table has not had love for months
16:12ekyleyes
16:12marcoahah
16:12marcookok
16:13ekyleso, I am working on getting coverage back up this week
16:13ekylemarco: I will tell you when it is ready for your browsing
16:14ekylemarco: both tables have the same data format
16:17marcoekyle: ok, cool
16:17marcoekyle: I can use codecov.io in the meantime
17:07ekylemarco: there is some old (june 26) records in ActiveData, if you want to use those:
17:07ekyle https://activedata.allizom.org/tools/query.html#query_id=5I0mKhmj
17:08ekylemarco: but that is a summary....
17:09ekylemarco: https://activedata.allizom.org/tools/query.html#query_id=qsbf1x8O
17:10marcoekyle: what is "missing":"test.url" for?
17:10ekylemarco: the record that is missing the test.url is the one that covers all tests
17:11ekylemarco: I forget if we need to filter on that still; it was long ago
17:12ekylemarco: yes, probably do not need it
17:14ekylemarco: I do not know why there are two records though, that should be fixed
17:30bdahlover in headless browsing we're trying to write some tests for a feature we're adding to firefox for taking screenshots of pages from the command line ( e.g. firefox --headless --screenshot). anyone have any thoughts on what test suite we could use to test it or will we need to add something new?
17:30bdahlato: maybe you have thoughts ^
17:50bwintonbdahl: I'm no testing expert, but I would probably try using Mochitests first, and then perhaps switch over to Marionette/Puppeteer if that didn't work.
17:51bdahlbwinton: does mochitest support running the firefox exe with different arguments?
17:51bdahlor launching another process
17:52bwintonbdahl: I'm not sure, but I would guess not, which is why you might switch over to Marionette.
17:52bwintonHopefully someone who actually knows our various test frameworks will also reply. ;)
17:53bdahlbwinton: it's a bit of a strange test, because we just want to run a binary and then make sure an image file was created, we don't actually want to start marionette to control the headless browser
17:55bwintonbdahl: Yeah, it is a bit of an odd case. :) But you can start up and shut down Firefoxen with Marionette, so it may still work for you.
17:55marcobdahl: IIRC we run a similar test (running a binary) for the webruntime in the past
17:56bdahlmarco: any links?
17:58marcobdahl: https://hg.mozilla.org/mozilla-central/file/31655275a522/toolkit/webapps/tests/test_hosted_checkforupdates_from_webapp_runtime.xul
18:04bdahlmarco: thx. i'm still kind of surprised there are no tests for the various command line arguments that firefox supports
18:53whimboobdahl: it would be easy to do with Marionette
18:54whimboobut if you find something which works with Mochitests, its also fine
18:55bdahlwhimboo: bdahse is trying marionette first. i'm not sure how marionette will like the browser immediately quitting though since `--screenshot` exits right after dom content loaded
18:56whimboobdahl: there is a way to instruct Marionette to use a callback for quitting firefox
18:56whimboooh or is that right after startup?
18:59bdahlwhimboo: it's right after startup and it doesn't follow the same path of creating the full browser, just creating a WindowlessBrowser
18:59whimbooit would mean that we run into an IOError
19:00whimboois that happening after session restored?
19:00whimbooif yes then we marionette is even not fully initialized
19:26kevinjwhat is the webextensions equivalent (tests found in browser/components/extensions/test) equivalent for promiseBrowserState? (promiseBrowserState is not accessible in webextensions tests)
20:22ahalStandard8: sorry for the delay, something like this works: https://hg.mozilla.org/try/rev/17f78807ece65fa9432e1c4c7c8e6b7e0d692a70
20:23ahalyou can land your thing first and I'll rebase this on top afterwards
20:24ahalhopefully they can both extract to /build/node_modules without errors, but if not it's easily worked around
20:26Standard8ahal: ok great
20:26Standard8thanks for that
20:26Standard8Ill incorporate it later or in the morning (my time)
20:27ahalnp, unrelatedly, I discovered that the pre-push hook stops all changes that modify a .eslintrc file
20:29Standard8ouch
20:44Standard8ahal: oh, and it looks like the hooks dont work for git due to permissions...
20:44ahaldo they need a +x?
20:44ahalwas hoping that would be preserved
20:45Standard8yes they do, it should have been
20:45ahaloh, I thought I landed it with +x but maybe I forgot to do that
20:47ahalbug 1380135 fyi
20:47bugbotBug https://bugzilla.mozilla.org/show_bug.cgi?id=1380135 Lint, normal, nobody, NEW , Mozlint "pre-push" hook prevents any change that modifies an ignored eslint file
20:47ahaldue to the ignored by default warning
20:47Standard8ahal: quiet
20:47Standard8err I think
20:47ahalthat'll work, but also suppress all other warnings
20:47Standard8yep, --quiet
20:47ahalis that wanted?
20:48Standard8not sure, generally we dont allow non-errors in the code
20:48Standard8well, we dont allow warnings for rules
20:48ahalStandard8: I've been meaning to ask you about that actually
20:48Standard8since everyone ignores them
20:48ahalshould we make warnings fail the task?
20:48ahalthen maybe "warn" == it's ok to ignore this via config
20:49ahaland "error" == this is bad, fix it
20:49ahalor something
20:51ahalseems slightly more useful than not using warnings at all
20:52Standard8well, I dont want warnings at all
20:52Standard8people just ignore them
20:52Standard8its a bit like random oranges
20:52ahalStandard8: right, but I'm saying what if we made it so they can't ignore them?
20:52ahali.e they get backed out
20:53ahalmight be a bit of friction at first, but they would clue in quickly
20:53Standard8well at the moment we have zero warnings anyway
20:53ahalyeah, I guess that works too
20:53Standard8Im half-pondering a test to ensure that no rules are set to warn
20:54ahalfwiw, eslint is the only linter that doesn't fail on warnings in automation
20:54ahalbut anyway, I guess it's not a big deal
20:55ahaleither disallow warning outright or make warnings turn the task orange is kind of the same thing
20:57Standard8yeah true
20:58Standard8ahal: so whos dealing with the permissions issue?
20:59ahalStandard8: I'll piggyback it off bug 1380135 as they are both hook related fixes
20:59bugbotBug https://bugzilla.mozilla.org/show_bug.cgi?id=1380135 Lint, normal, nobody, NEW , Mozlint "pre-push" hook prevents any change that modifies an ignored eslint file
20:59Standard8ok cool
21:01ahalStandard8: so should I just always add --quiet when running eslint?
21:01ahalso if a rule is set to "warn" it'll never show results via |mach lint|
21:01Standard8ahal: maybe just the hooks for now
21:01ahalok
21:01Standard8I think it might be useful for developers to know if theyre manually trying to lint a file that is ignored
21:05marcoekyle: gmierz|afk: do you remember the bug # where we enabled ccov builds for every mozilla-central push?
21:06marcoI'm thinking we could avoid the build for pushes like this: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=6fec4855b5345eb63fef57089e61829b88f5f4eb
21:06marcothey are pretty common (at least once a day) and don't really change anything
21:07ekylemarco: what are they
21:07ekyle?
21:08marcoekyle: they look like automated updates to some SSL information
21:09ekylethe per push coverage bug is hard to find...
21:32Manishearthjgraham: btw, something was weird with this try push
21:32Manishearthhttps://treeherder.mozilla.org/#/jobs?repo=try&revision=caf18181be608dd33acbee3f70898327c0fa924f
21:32Manishearththe try push commit itself has a null diff but it says that it touched some files
21:32Manishearthhttps://hg.mozilla.org/try/rev/caf18181be608dd33acbee3f70898327c0fa924f
21:33Manishearthadditionally, it actually built the code without the latest patch, even though it's there in the hg history
21:33marcoekyle: looks like TestPrintf is not executed in normal builds either
21:34ekylemarco: thank you for looking at that, I wonder if it is skipped (or part of a long lost test suite)
21:35marcoekyle: it looks like the test was added in May of this year
21:35marcohttps://bugzilla.mozilla.org/show_bug.cgi?id=1334276
21:35bugbotBug 1334276: XPCOM, normal, ttromey, RESOLVED FIXED, add unit tests for Smprintf et al
21:35marcoekyle: oh, and it looks like it was written to get coverage to a higher level
21:36marcoso it was running at some point
21:36ekylemarco: that explains why ttromey is so interested in it running
21:36ekyle:)
21:41ekylemarco: the grcov artifact already has source file path translation done?
21:41marcoekyle: not yet, it will soon
21:42ekylemarco: is that the post-step? or is it part of the end-of-test?
21:42ekylemarco: (I do not know what you plan to call the task that runs at the end of all test jobs)
21:42ekylemarco: ("post-step"=="post-task"???)
21:52ekylemarco: the grcov artifact is sweet! thank you!
21:52marcoekyle: it's part of the test task itself
21:52marcothe only thing that will not be part of the test task itself but will be part of the post task is the JSVM LCOV rewriting
21:53marcobecause it requires a source checkout
12 Jul 2017
No messages
   
Last message: 14 days and 17 hours ago