Andrew Halberstadt [:ahal] Assignee
	Description • 10 years ago

Linux64 code coverage is running in Cedar:
https://tbpl.mozilla.org/?tree=Cedar

Currently it shows up on the same line as Linux64 Opt which means that all the test jobs get interleaved with the non-code coverage variant. We should split them onto their own line.

	Ryan VanderMeulen [:RyanVM]
	Comment 1 • 10 years ago

Bug 911237 shows how this was done for ASAN builds. Are we doing this for opt+debug?

	Andrew Halberstadt [:ahal] Assignee
	Comment 2 • 10 years ago

Eventually we'll want to enable both opt+debug as well as additional platforms. For now it's just Linux64 Opt I'm focusing on. So I can either add them both now, or add additional ones later as needed.

	Andrew Halberstadt [:ahal] Assignee
	Comment 3 • 10 years ago

Ok, I'm thoroughly confused by tbpl. This is the result of trying to copy bug 911237. I'm not sure if this works or how to test it though.

	Andrew Halberstadt [:ahal] Assignee
	Comment 4 • 10 years ago

I'm also worried that 'cc' won't be unique enough in the regex matching. Not sure what else to do about it though.

	Ryan VanderMeulen [:RyanVM]
	Comment 5 • 10 years ago

Comment on attachment 8480842 [details] [diff] [review]
add_cc_flavour

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

You should only need the middle 2 hunks in Data.js and the one change in UserInterface.js for now. We had to special-case the ASAN stuff due to having to show both opt and debug on the same line.

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 10 years ago

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #5)
> You should only need the middle 2 hunks in Data.js and the one change in
> UserInterface.js for now. We had to special-case the ASAN stuff due to
> having to show both opt and debug on the same line.

s/middle 2/first 2 :)

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 10 years ago

Also, for testing, you can load index.html locally and verify that things display correct on Cedar. Appending &debug=1 will give you error output to the console if there are problems.

I didn't test your patch locally due to the aforementioned issues in comment 5, but I'll take a closer look at the updated patch.

	Andrew Halberstadt [:ahal] Assignee
	Comment 8 • 10 years ago

I don't really care about TBPL anymore. I'll look into getting this fixed in treeherder instead.

	Andrew Halberstadt [:ahal] Assignee
	Comment 9 • 10 years ago

I feel like there's no way that this is all that's needed, but I can't seem to find anything else that might need to get added. Cam, am I on the right track? What's the best way to test this?

If you're missing context, I'm trying to get the code coverage builds on cedar (the orange B) to show up on their own line:
https://treeherder.mozilla.org/ui/#/jobs?repo=cedar&filter-searchStr=Code%20Coverage

	Cameron Dawson [:camd]
	Comment 10 • 10 years ago

Talked about this on IRC with ahal.  He's going to create a test with the new buildername to make sure they run ok.  Then he'll update the UI file that places "code coverage" as an opt in the right order (separate PR for the UI).  But we'll test this on dev before merging to master.

	Ed Morley [:emorley]
	Comment 11 • 10 years ago

This is now a dupe of bug 1063007.

I'm happy to make the treeherder changes for this if it's easier - the main thing is we need:
a) A finalised buildername
b) To decide how we're displaying these - ie as their own row like ASan

Seems like we have both now, right? :-)

	Andrew Halberstadt [:ahal] Assignee
	Comment 12 • 10 years ago

Oh sorry, I didn't realize you'd already filed another bug for this. I don't really care if you resolve this bug or that one as a dupe. I updated the PR with the tests that Cam was talking about: https://github.com/mozilla/treeherder-service/pull/297

I'm not sure if anything needs updating in treeherder-ui or not. I grepped for 'asan' (which is set up similarly) and the only results were either under 'webapp/app/sample-data' or 'webabb/test/mock'. Do I need to add tests for code coverage here as well?

	Ed Morley [:emorley]
	Comment 14 • 10 years ago

(In reply to Andrew Halberstadt [:ahal] from comment #12)
> I'm not sure if anything needs updating in treeherder-ui or not. I grepped
> for 'asan' (which is set up similarly) and the only results were either
> under 'webapp/app/sample-data' or 'webabb/test/mock'. Do I need to add tests
> for code coverage here as well?

No I think this should be fine as is :-)

	Treeherder GitHub Bugbot
	Comment 15 • 10 years ago

Commits pushed to master at https://github.com/mozilla/treeherder-service

https://github.com/mozilla/treeherder-service/commit/b8d9084b61904825f0ddcc4450fbf44562813b8d
Bug 1056236 - Add a new code coverage build type

https://github.com/mozilla/treeherder-service/commit/84a9f8c1a7c1a3ad69467fa1ad638ff021b58633
Merge pull request #297 from ahal/code_coverage_platform_name

Bug 1056236 - Add a new code coverage build type