mozilla :: #treeherder

17 Mar 2017
02:46cwillEli: sorry, I went back to work so I'm out on Tues & Turs - thank you SO MUCH for looking at that problem!!
02:47cwillI'll fix it up and update the PR for tomorrow :)
02:47Elicwill: sounds good!
10:44jgrahamI really wish I could title treeherder tabs
10:44jgrahamI end up with half a dozen try jobs open and no way to tell which is which
11:00emorleyjgraham: has this broken?
11:04jgrahamemorley: Not sure, but with e.g. I just see "[0] Try" which isn't at all useful (I would call it "Coverage", which it can't tell from the commit)
11:05jgrahamOh, wrong link :)
11:05jgrahamBut same problem
11:57emorleyjgraham: would you mind filing a bug - it appears that feature has broken
11:58emorleyor at least isn't working like it did in TBPL (where the title would have included "Add six to wptserve dependencies")
11:59jgrahamemorley: Sure
11:59emorleyand mark it as blocking bug 1045602
11:59firebot FIXED, The page title for individual revisions should include details (port bug 1020919 to treeherder)
13:17KWierso|afkemorley: ooh, this can be tested in the new selenium tests, right?
13:18emorleyI presume so :-)
13:18emorleythough I imagine the issue is likely something that could be unit tested too
14:20wlach|afkemorley: I think selenium makes more sense here, the unit tests are more useful for testing the dataflow between angular controllers and services
14:21emorleyto be clearer, I think the first step should be unit-testing the regex matching commit message strings as a unit test
14:21emorleyand I agree that for the overall solution, a Selenium test makes sense
14:21wlachKWierso|brb: I think a selenium test would be like 5 lines or so, you would just need to navigate to a list of resultsets and an individual commit, and verify the title string for each
14:22wlachyeah, for sure, that would be a useful unit test too
14:22wlachthough I think the selenium test is more likely to catch bugs in practice
14:23emorleywell it's more that we only really need one selenium test to check that title setting and capturing of commit message works, but then half a dozen unit tests of the ~`extract_title_from_commit_messages()` part
14:23emorleyI think having half a dozen selenium tests for this would be overkill
14:24wlachright, it's more that we just want to test that setting a special commit message *at all* doesn't break
14:57herokubotdeployed 10430f3 to treeherder-prototype
14:57herokubotdeployed 10430f3 to treeherder-stage
15:21KWiersowlach: easiest way to run the selenium tests with me on Windows? push to a PR and let travis deal with it?
15:22wlachKWierso: yeah there's a bit of vagrant setup that you need to do to run locally, I should write that down somewhere...
15:22wlachKWierso: so yeah, maybe travis is easiest for now
15:25wlachI'll try to document the process later today
16:11KWiersowlach: any idea if push data is loaded in the selenium tests? I'm trying to do something like this
16:11KWiersobut it's just timing out without finding that element
16:12KWiersoand I don't know if there's something else I need to be doing to pull in some sample pushes
16:14KWierso(it works when I try | By.ID, 'repoLabel' | like your first test, but that's from the site header that immediately loads prior to any push data loading
16:30wlachKWierso: yeah you'll need to add a push or two
16:30wlachKWierso: let me dig up an example
16:32wlachKWierso: something as simple as this should work
18:02herokubotdeployed e328bff to treeherder-stage
18:03herokubotdeployed e328bff to treeherder-prototype
18:11KWiersowlach: hrm, there must be more to it that I'm not seeing (or I'm cribbing a bit too much from that example without changing something?)
18:12KWiersowith I'm seeing
18:35herokubotdeployed cfc8870 to treeherder-stage
18:35herokubotdeployed cfc8870 to treeherder-prototype
19:15wlachKWierso: sorry have been distracted with other stuff, looking now
19:15KWiersoheh, same
19:16wlachKWierso: try removing the initial_data fixture. we should only need test_repository for this one
19:17wlachinitial_data includes the full repository list
19:25KWiersowlach: well, that fixed that error, but now I'm back to timing out trying to find element by CLASS_NAME
19:27herokubotdeployed 2ad2731 to treeherder-stage
19:27wlachKWierso: it is possible the test is hanging due to insufficient data
19:27herokubotdeployed 2ad2731 to treeherder-prototype
19:28KWiersowlach: wonder if I could grab a screenshot of the page at the point it times out
19:28KWiersosee if it's even doing what I hope it's doing
19:28wlachKWierso: yes! selenium has exactly that option
19:28wlachalthough actually
19:29wlachI'll bet you anything the right thing is to restore the initial_data fixture, remove the test_repository fixture, then hang the push off of a look of the mozilla central repository
19:29wlacha lookup
19:32KWiersowlach: so...
20:02KWiersowlach: so if I take a screenshot before the call to wait, here's what I see
20:03KWiersoI'll switch back to waiting for the repoLabel ID and see what the screenshot looks like
20:11wlachKWierso: hmm interesting looks like it's not displaying the push
20:23KWiersowlach: and after the wait,
20:23KWiersowhich is this code
20:25emorleyI'm heading out now - have a good weekend everyone! :-)
20:25wlachyou too emorley!
20:25KWiersobye, emorley
20:25wlachKWierso: hmm
20:26KWiersomaybe it's rejecting the Push for being invalid?
20:26wlachyou would have gotten an error if that were the case
20:26wlachoh huh
20:28wlachKWierso: it's failing to create the commit object
20:28wlacher the push object
20:29KWiersothat was me just now adding a "comment" to the Push
20:29* KWierso blindly stabs at this a bit before looking up what goes into a Push
20:29wlachKWierso: the model is defined in treeherder/treeherder/
20:29wlachKWierso: you are 99% of the way there I am sure
20:30KWiersowlach: do I need to create a Commit, which I then add to the Push?
20:33wlachKWierso: I don't *think* you do
20:33KWiersobecause otherwise, none of this specifies a commit message :\
20:34wlachah yes
20:34KWiersomaybe it's "comments", not "comment"
20:34* KWierso tries that
20:34wlachCommit.objects.create(push=push, revision=revision, author="", comments="XXX")
20:35wlachKWierso: no, push has no comments field. I just checked
20:37KWiersocrap, never imported Commit
20:40wlachKWierso: let me figure out how to run these tests locally again, it's not hard
20:42KWiersowlach: ooh, and I'll need to wait even more after that, commit/push gets created, too, won't I?
20:43wlachKWierso: no, no need to wait, the state should be updated immediately. I would also do that at the beginning of the test
20:45KWiersohere's what the test does, afaict:
20:45KWiersoload the page, create the push object, create the commit object referring to that push, wait for the repoLabel element to be located in-page, take screenshot, assert and show the screenshot
20:45KWiersounless you're saying to create the push/commit before the initial page load?
20:45wlachyeah, I would do that first
20:45wlachcreate the state, then load the page
20:50KWiersowlach: hey, that worked!
20:50wlachKWierso: oh good!
20:51* KWierso wonders if he can switch back to waiting on CLASS_NAME now
20:52wlachyeah whatever we're doing in the old test should be enough
20:53wlachthen, later in the test, navigate to the page with the revision specified and verify the title is what you expect
20:53KWiersoguess I'll also need to mock up some jobs to get the percentage of completed jobs?
20:53KWiersoor call it good at just getting the commit message
20:54wlachI would just call it good at the commit message
20:54wlachcreating mock job data is another can of worms, which we should tackle someday but maybe not today
20:54KWiersoall of this because the only unittests are for jobs.js and not main.js :P
20:55KWiersoand I didn't want to have to create that myself
21:05wlachwell I think integration tests are always good
21:10KWiersowlach: wonder if there's some global selenium "take and print screenshot on failure" thing we could turn on
21:10KWiersomy test is passing now, but it'd be nice to not have to manually add a screenshot into the debugging process
21:12KWiersoha, and now the ui-tests, that I didn't touch are failing on my PR because > The command "curl -sSfL | bash" failed and exited with 1 during .
21:12KWiersogpg: no valid OpenPGP data found.
21:17* KWierso hits the ui-tests with a stick
21:18KWierso|afkwlach: thanks a lot for the help!
21:22wlachKWierso|afk: that's a great idea re: take screenshot on failure
18 Mar 2017
No messages
Last message: 188 days and 13 hours ago