Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing the Remove sibling Build on allTestsInfo page #878 #881

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Sangyoon21
Copy link
Contributor

Fix issue #880

I had removed the sibling Build (build 77 -74) on allTestsInfo page
It looks like the below image

Screenshot 2024-06-09 at 5 07 32 PM

Copy link

netlify bot commented Jun 9, 2024

Deploy Preview for eclipsefdn-adoptium-trss ready!

Name Link
🔨 Latest commit 118057d
🔍 Latest deploy log https://app.netlify.com/sites/eclipsefdn-adoptium-trss/deploys/6674a947100aed0008b62923
😎 Deploy Preview https://deploy-preview-881--eclipsefdn-adoptium-trss.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -84,7 +84,10 @@ const Build = () => {
return rt;
});

setParents(fetchedParents);
// Filter out siblings builds (Build #77 - 74)
const filteredParents = fetchedParents.filter(parent => parent.buildNum > 77);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to have a generic solution. Not for any specific build numbers.
In this case, I think we do not need parents data.
To quickly test this, we can try to remove parents={parents} at

<TestTable title={'Tests'} testData={testData} parents={parents} />
and confirm the behavior. Then we can remove parents data completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have removed parents={parents} part from line 167.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing parents={parents} was suggested to be a quick test, not the full solution.
Currently, removing parents={parents} results the allTestsInfo page to crash TypeError: Cannot read properties of undefined (reading 'buildName'). We need to investigate more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh okay i understood it now, i would put the parents={parents} back and try to fix the other parts. But i have a question of is there are another page like https://trss.adoptium.net/allTestsInfo?buildId=66582ddf879917006e217d83&limit=5&hasChildren=true where I could also test on? Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sangyoon21 Yes, it would be the case like the page you posted. We just want to show the first build (78 Failed in this case, but not 77 7 6 75 etc.).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the test, in your local TRSS, you can add monitoring of this build evaluation-openjdk17-pipeline(you can refer the current monitoring list for the format), the data will shown after downloaded to your local TRSS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay thank you for the response! i would try to fix this!

@llxia llxia requested a review from LongyuZhang June 10, 2024 12:24
@karianna
Copy link
Contributor

FYI - CI will fail until CodeQL action is updated.

@karianna karianna requested a review from llxia June 24, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants