adoptium / adoptium/aqa-test-tools

Move Duplicate Code for Perf Graphs to Common Utils Library

Open
#118 3 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Jupyter Notebook
Stars
33
Forks
97
Avg merge
7h 9m
Merged PRs (30d)
5

Description

### Problem Description

Currently, we have 3 Perf widgets for Dashboard: DayTrader, ODM and SPECjbb2015. There is significant duplication of code between those 3 widgets since we just copied and modified the code from the first widget instead of using a common library while adding a new widget every time.

All perf graphs have the same purpose of displaying perf results for different benchmarks run on different platforms. Besides some specific data, everything else is the same among those widgets as shown below in the screenshots.

![image](https://user-images.githubusercontent.com/10422081/59708594-14868d00-91d3-11e9-91ff-ebb8727856cc.png)

![image](https://user-images.githubusercontent.com/10422081/59707793-5c0c1980-91d1-11e9-91f4-7221864034d3.png)

The graphs would have some minor feature difference because some of the features added by https://github.com/AdoptOpenJDK/openjdk-test-tools/pull/84, where not extended to all 3 perf widgets.

As a result, it's not easy to add new widgets for new benchmarks without duplicating code from some existing widget.

### Proposed Changes

We should use a library to keep the common code, in order to avoid duplicating code for different benchmark widgets. For example, currently, `utils.js` just has one function `parseSHA`, which is used in multiple widgets but there is still enough scope to clean up code by moving common code to this library.

https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/524d16e3784c17f4af6cee75f9105eb929397792/test-result-summary-client/src/Dashboard/Widgets/Graph/utils.js#L1-L2

https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/524d16e3784c17f4af6cee75f9105eb929397792/test-result-summary-client/src/Dashboard/Widgets/Graph/ODM.jsx#L159-L160

https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/524d16e3784c17f4af6cee75f9105eb929397792/test-result-summary-client/src/Dashboard/Widgets/Graph/DayTrader3.jsx#L151-L152

Contributor guide

Open the contributing guide

Research direction

Start by comparing test-result-summary-client/src/Dashboard/Widgets/Graph/ODM.jsx and DayTrader3.jsx with Graph/utils.js, then inspect the corresponding SPECjbb2015 widget to identify duplicated graph behavior. Move genuinely shared graph code into the common utility library while preserving benchmark-specific differences, and verify that all three dashboard widgets still display their performance results correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.