Bug 1539232 - Switch Perfherder to react-router by sarah-clements · Pull Request #5379 · mozilla/treeherder

sarah-clements · 2019-09-17T23:25:08Z

This is the final react conversion pr - woohoo!

In addition to setting up the routes, I've made these other changes:

Using top-level of app as a cache for projects, frameworks, alerts data and compare data
Cleanup files and move constants to dedicated perfherder file
Removed angular-related libraries and bump down the neutrino entry and asset limits

@ionutgoldan it'd be worth you giving all the views a quick test. I unfortunately had a lot of squashing of commits to do during the rebase and I want to make sure the new retrigger action is working as expected. I'd also like to ensure that the fetched data that's stored from the alerts view (when navigating back from an individual alert), and the fetched data that's stored from the compare view (when navigating back from the comparesubtest view) works as expected and doesn't cause any issues. I've launched it to prototype.

FYI @karlht and @davehunt

sarah-clements · 2019-09-17T23:51:15Z

@camd There's a selenium test that's failing - this test for switching between Treeherder and Perfherder in the dropdown menu. Do you think this is worth trying to get working or were you planning to rewrite at some point? I've rewritten the other tests for Perfherder using react-testing-library so this is the only remnant of Perfherder selenium tests.

codecov-io · 2019-09-19T18:08:40Z

Codecov Report

Merging #5379 into master will increase coverage by 0.06%.
The diff coverage is 11.86%.

@@            Coverage Diff             @@
##           master    #5379      +/-   ##
==========================================
+ Coverage   36.94%   37.01%   +0.06%     
==========================================
  Files         190      190              
  Lines        5995     6001       +6     
  Branches     1331     1341      +10     
==========================================
+ Hits         2215     2221       +6     
- Misses       3490     3491       +1     
+ Partials      290      289       -1

Impacted Files	Coverage Δ
ui/intermittent-failures/helpers.js	`0% <ø> (ø)`	⬆️
ui/perfherder/graphs/GraphsViewControls.jsx	`31.25% <ø> (-4.05%)`	⬇️
ui/perfherder/compare/CompareTableControls.jsx	`94.73% <ø> (ø)`	⬆️
ui/perfherder/alerts/AlertTableRow.jsx	`78.08% <ø> (ø)`	⬆️
ui/perfherder/compare/ReplicatesGraph.jsx	`91.89% <ø> (ø)`	⬆️
ui/perfherder/alerts/DownstreamSummary.jsx	`5.55% <ø> (ø)`	⬆️
ui/intermittent-failures/MainView.jsx	`0% <ø> (ø)`	⬆️
ui/job-view/headerbars/PrimaryNavBar.jsx	`83.33% <ø> (ø)`	⬆️
ui/shared/HelpMenu.jsx	`100% <ø> (ø)`
ui/helpers/constants.js	`100% <ø> (ø)`	⬆️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a26730...96a58c0. Read the comment docs.

sarah-clements · 2019-09-19T18:27:07Z

@camd There's a selenium test that's failing - this test for switching between Treeherder and Perfherder in the dropdown menu. Do you think this is worth trying to get working or were you planning to rewrite at some point? I've rewritten the other tests for Perfherder using react-testing-library so this is the only remnant of Perfherder selenium tests.

Cameron's #5383 pr fixed this problem.

ionutgoldan · 2019-09-20T14:12:30Z

I only managed to see that the permalinks from the Compare view no longer work as expected.
Prior to this PR, clicking on a permalink would put the item at the top of the page. Now, that item is no longer visible.

STR

Go to a Compare view with some data (e.g.)
Click on the # permalink next to raptor-ares6-chromium opt.

Expected:
The raptor-ares6-chromium opt header should show up at the top of the page.

What we actually get:
raptor-ares6-chromium opt header isn't visible anymore. At least, not entirely.

ionutgoldan · 2019-09-20T14:18:28Z

@alexandru-io @bebef1987 could you help me out on testing this PR next Monday?
If we have more eyes on this, we can spot more bugs.

sarah-clements · 2019-09-20T17:27:01Z

I only managed to see that the permalinks from the Compare view no longer work as expected.
Prior to this PR, clicking on a permalink would put the item at the top of the page. Now, that item is no longer visible.

STR
1. Go to a Compare view with some data ([e.g.](https://treeherder-prototype.herokuapp.com/perf.html#/compare?originalProject=mozilla-central&originalRevision=a3a081ae714f1123bdc23c9d9ef53dfaa783a8de&newProject=mozilla-central&newRevision=6b93a83735ed3ab3b57b46c1b768814b1a1af5d6&framework=10))

2. Click on the [# permalink](https://treeherder-prototype.herokuapp.com/perf.html#/compare?originalProject=mozilla-central&originalRevision=a3a081ae714f1123bdc23c9d9ef53dfaa783a8de&newProject=mozilla-central&newRevision=6b93a83735ed3ab3b57b46c1b768814b1a1af5d6&framework=10#table-header-566498077) next to `raptor-ares6-chromium opt`.
Expected:
The raptor-ares6-chromium opt header should show up at the top of the page.

What we actually get:
raptor-ares6-chromium opt header isn't visible anymore. At least, not entirely.

Ah, I know why it's doing that. It's because of how the top menu navigation is overlapping the view. So the permalink is still working correctly, I'll just need to fix how the menu is displayed.

camd

Looks great! Wow, you're done! :) Nice work. Take the weekend off...
I didn't test this myself, I must admit. I'll trust to your testing here.

ui/perfherder/compare/CompareTable.jsx

ionutgoldan · 2019-09-23T06:38:18Z

I am now resuming the testing on this PR.

ionutgoldan · 2019-09-23T06:44:16Z

Bug #2
Alerts navbar item has weird behavior, on the 2nd click.

STR

Go to default landing view of Perfherder. (which is Alerts view)
Click on Compare navbar item, from top-left.
After it loads, click on Alerts navbar item & wait until it fully loads.
Click on Alerts navbar item one more time.

Expected results:
A page similar to that from step #3. That is a page with alerts summaries

What we actually get:
A page with no alert summaries, which states "No alerts to show".

This bug is specific to this PR.

ionutgoldan · 2019-09-23T07:52:10Z

Bug #3
Permalinks no longer visible in Compare subtest view

STR

Go to a Compare view with data. (e.g.)
Follow one of the available subtests hyperlinks. (e.g.)

Expected results:
Table header & all subtests display a # permalinks.

What we actually get:
No # permalinks are displayed.

ionutgoldan · 2019-09-23T08:29:57Z

Bug #4
Retrigger buttons from Compare view & Compare subtest view are broken.

STR

Go to Compare view with data. (e.g.)
Click on the retrigger button under the # Runs column from right side, from the raptor-ares6-chromium opt table, the linux64-shippable row.

Expected results:
No console errors & new AC(rt) jobs are pending for base & new on Treeherder.

What we actually get:
TypeError: s is not a function console error & no retriggers are done Treeherder.

ionutgoldan · 2019-09-23T08:42:45Z

Bug #5
Pagination from bottom of page still visible when checking out a single summary.

STR

Go to random Alerts view. (e.g.)
Checkout a random summary from the available ones there. (e.g.)

Expected results:
No pagination at the bottom of page.

What we actually get:
A gray & squared 1 page at the bottom of page.

Note: this is a preexisting bug, which I've found just now.

ionutgoldan · 2019-09-23T08:58:15Z

Bug #6
hideDwnToInv=1 now shows up in the URL of checkout out summaries.

STR

Go to random Alerts view. (e.g.)
Randomly look at 2 alert summaries, from the same repository. (e.g. 1st summary, 2nd summary)
Reassigned all alerts from the 1st summary to the 2nd summary.
Checkout to the 1st alert summary page. (e.g.)

Expected results:
The Hide downstream / reassigned to / invalid checkbox isn't enabled, so that user sees all alerts stroke through.

What we actually get:
The Hide downstream / reassigned to / invalid is enabled.

marianrai · 2019-09-23T09:55:37Z

Login issue on Firefox and Chrome;

STR

Go to https://treeherder-prototype.herokuapp.com/perf.html
Logged in successfully
After step2 i see a blue alert info on the top : "You must be logged into perfherder/treeherder and be a sheriff to make changes". On top right i can see my name, but i'm not really logged in.
I can't use the checkboxes on alert summaries nor open new alerts from graphs.
Cleared cache, cookies, restarted browsers - issue persists.

Expected results:
After the login i should have full functionality on perfherder.

What we get instead:
Perfherder acts as i am not logged in.
In Chrome i can see this error in console:
"Login issuie in heroku perfherder
AuthService.js:66 Could not renew login: Object
_renewAuth @ AuthService.js:66"

Video with the issue from Alex: https://mozilla.zoom.us/recording/play/g5UjN_GDTxhejx0bepu3XyRSFa94S4_-wqFQyY652kSrTXPwMeqzHTgwp0ke4c7T?continueMode=true

ionutgoldan · 2019-09-23T13:34:00Z

Login issue on Firefox and Chrome;

STR
1. Go to https://treeherder-prototype.herokuapp.com/perf.html

2. Logged in successfully

3. After step2 i see a blue alert info on the top : "You must be logged into perfherder/treeherder and be a sheriff to make changes". On top right i can see my name, but i'm not really logged in.

4. I can't use the checkboxes on alert summaries nor open new alerts from graphs.

5. Cleared cache, cookies, restarted browsers - issue persists.
Expected results:
After the login i should have full functionality on perfherder.

What we get instead:
Perfherder acts as i am not logged in.
In Chrome i can see this error in console:
"Login issuie in heroku perfherder
AuthService.js:66 Could not renew login: Object
_renewAuth @ AuthService.js:66"

Video with the issue from Alex: https://mozilla.zoom.us/recording/play/g5UjN_GDTxhejx0bepu3XyRSFa94S4_-wqFQyY652kSrTXPwMeqzHTgwp0ke4c7T?continueMode=true

This is a false alarm. You were missing the is_staff flag set on the prototype database. I've addressed that and you should no longer experience this issue.

sarah-clements · 2019-09-24T00:36:38Z

Bug #2
Alerts navbar item has weird behavior, on the 2nd click.

STR
1. Go to default landing view of Perfherder. _(which is Alerts view)_

2. Click on `Compare` navbar item, from top-left.

3. After it loads, click on `Alerts` navbar item & wait until it fully loads.

4. Click on `Alerts` navbar item one more time.
Expected results:
A page similar to that from step #3. That is a page with alerts summaries

What we actually get:
A page with no alert summaries, which states "No alerts to show".

This bug is specific to this PR.

Ok, interesting, thanks for catching it - that's been fixed. Why would you click on alerts twice though?

sarah-clements · 2019-09-24T00:38:15Z

@ionutgoldan Btw, when you write #<some number> it links to old Treeherder pull requests and shows up as referencing that bug (click on your Bug #2 for example so it'd be best to use different notation :)

sarah-clements · 2019-09-24T02:44:37Z

Thanks for the feedback @ionutgoldan - I'll continue working on these fixes tomorrow and give it a good last test before merging.

ionutgoldan · 2019-09-25T08:23:57Z

@ionutgoldan Btw, when you write #<some number> it links to old Treeherder pull requests and shows up as referencing that bug (click on your Bug #2 for example so it'd be best to use different notation :)

Ok. Will use a different notation from now on. Thanks!

alexandru-io · 2019-09-25T09:25:13Z

Bug # 8
Data points for new test added in graph with button Add test data drawn with white.

STR

Click the graph button of an alert
Click Add test data and select the same test but on another platform (linux-64-shippable in this case)
See that the datapoints are drawn white. (e.g.) Recording here

Expected results:
The data points of the added hould be drawn in a different color than the background of the graph.

What we actually get:
The data points of the added test are drawn with the color of graph background.
Note that after closing the tab and opening another one, the added test is visible. This means that once in a while the color picked up by the graph to plot is white.

sarah-clements · 2019-09-25T20:56:51Z

Bug #5
Pagination from bottom of page still visible when checking out a single summary.

STR
1. Go to random `Alerts` view. ([e.g.](https://treeherder-prototype.herokuapp.com/perf.html#/alerts?hideDwnToInv=1&framework=6))

2. Checkout a random summary from the available ones there. ([e.g.](https://treeherder-prototype.herokuapp.com/perf.html#/alerts?id=22078))
Expected results:
No pagination at the bottom of page.

What we actually get:
A gray & squared 1 page at the bottom of page.

Note: this is a preexisting bug, which I've found just now.

I don't see this as a bug per say - technically it is the the one and only page. However, if you would like to improve upon the pagination please feel free.

davehunt · 2019-09-25T21:04:06Z

Note: this is a preexisting bug, which I've found just now.

Let's file any pre-existing bugs in bugzilla and not block this patch on them.

sarah-clements · 2019-09-25T21:11:05Z

Bug # 8
Data points for new test added in graph with button Add test data drawn with white.

STR
1. Click the graph button of an alert

2. Click **Add test data** and select the same test but on another platform (linux-64-shippable in this case)

3. See that the datapoints are drawn white. ([e.g.](https://treeherder.mozilla.org/perf.html#/graphs?timerange=1209600&series=autoland,1922714,1,1&series=autoland,1927614,1,1&highlightAlerts=1)) [Recording here](https://mozilla.zoom.us/recording/play/fTDrKg1yPwMyfqyz0Z6gAgI1kw1xq_upbPss9CrUfQ6VKJef6gqpOm3Niy5S3iPF?continueMode=true)
Expected results:
The data points of the added should be drawn in a different color than the background of the graph.

What we actually get:
The data points of the added test are drawn with the color of graph background.
Note that after closing the tab and opening another one, the added test is visible. This means that once in a while the color picked up by the graph to plot is white.

Thanks for reporting this but it doesn't have anything to do with this pr, which is for testing react router changes on prototype. Please file a bug with the video. I was able to reproduce this once - following the sequence from your video - but then wasn't able to reproduce it again. This might be an edge case.

Edit: I've been able to reproduce it again and will file a bug for it to save time.

Use top-level of app as a cache for projects, frameworks, alerts data and compare data Cleanup files and move constants to dedicated perfherder file Remove angular-related libraries and bump up the neutrino entry and asset limits

ionutgoldan · 2019-09-26T11:34:55Z

Note: this is a preexisting bug, which I've found just now.

Let's file any pre-existing bugs in bugzilla and not block this patch on them.

Will do.

edmorley · 2019-10-14T12:27:27Z

@sarah-clements @camd Congrats on finishing the AngularJS migration! :-)

camd · 2019-10-16T18:01:24Z

Thanks, @edmorley ! Great to hear from you! I hope you're well, old friend. :)

sarah-clements · 2019-10-16T23:44:35Z

Yes, @edmorley, it was a lot of work converting Perfherder and I'm glad it's finished! You should pop in the irc channel some time before it goes away... :)

sarah-clements force-pushed the react-router-conversion branch from a189ae7 to cf197ac Compare September 17, 2019 23:46

sarah-clements requested a review from camd September 17, 2019 23:51

sarah-clements temporarily deployed to treeherder-prototype September 17, 2019 23:59 Inactive

sarah-clements force-pushed the react-router-conversion branch from cf197ac to 62cff4f Compare September 19, 2019 17:57

sarah-clements temporarily deployed to treeherder-prototype September 19, 2019 19:13 Inactive

camd approved these changes Sep 20, 2019

View reviewed changes

ui/perfherder/compare/CompareTable.jsx Show resolved Hide resolved

sarah-clements added 2 commits September 25, 2019 14:19

Bug 1539232 - Switch Perfherder to react-router

37f52d7

Use top-level of app as a cache for projects, frameworks, alerts data and compare data Cleanup files and move constants to dedicated perfherder file Remove angular-related libraries and bump up the neutrino entry and asset limits

fixes based on feedback

96a58c0

sarah-clements force-pushed the react-router-conversion branch from 62cff4f to 96a58c0 Compare September 25, 2019 21:20

sarah-clements merged commit 59737af into master Sep 25, 2019

sarah-clements deleted the react-router-conversion branch September 25, 2019 22:15

Conversation

sarah-clements commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarah-clements commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sarah-clements commented Sep 19, 2019

Uh oh!

ionutgoldan commented Sep 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ionutgoldan commented Sep 20, 2019

Uh oh!

sarah-clements commented Sep 20, 2019

Uh oh!

camd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ionutgoldan commented Sep 23, 2019

Uh oh!

ionutgoldan commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ionutgoldan commented Sep 23, 2019

Uh oh!

ionutgoldan commented Sep 23, 2019

Uh oh!

ionutgoldan commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ionutgoldan commented Sep 23, 2019

Uh oh!

marianrai commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ionutgoldan commented Sep 23, 2019

Uh oh!

sarah-clements commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarah-clements commented Sep 24, 2019

Uh oh!

sarah-clements commented Sep 24, 2019

Uh oh!

ionutgoldan commented Sep 25, 2019

Uh oh!

alexandru-io commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarah-clements commented Sep 25, 2019

Uh oh!

davehunt commented Sep 25, 2019

Uh oh!

sarah-clements commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ionutgoldan commented Sep 26, 2019

Uh oh!

edmorley commented Oct 14, 2019

Uh oh!

camd commented Oct 16, 2019

Uh oh!

sarah-clements commented Oct 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

sarah-clements commented Sep 17, 2019 •

edited

Loading

sarah-clements commented Sep 17, 2019 •

edited

Loading

codecov-io commented Sep 19, 2019 •

edited

Loading

ionutgoldan commented Sep 20, 2019 •

edited

Loading

ionutgoldan commented Sep 23, 2019 •

edited

Loading

ionutgoldan commented Sep 23, 2019 •

edited

Loading

marianrai commented Sep 23, 2019 •

edited

Loading

sarah-clements commented Sep 24, 2019 •

edited

Loading

alexandru-io commented Sep 25, 2019 •

edited

Loading

sarah-clements commented Sep 25, 2019 •

edited

Loading