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

Fixed failed tests links' help prompt #38

Merged
merged 3 commits into from
Sep 26, 2024

Conversation

patrikcerbak
Copy link
Collaborator

Updated the failed tests links' help prompt to be accurate and 1:1 usable.

@patrikcerbak patrikcerbak requested a review from a team as a code owner September 2, 2024 18:55
<li><i>Base tool page:</i> list.html?=</li>
<li><i>Spliterator:</i> -</li>
<li><i>Tool arguments:</i> <code>-view=all-tests, -output=html, -fill, -track=%{TESTNAME} %{JOBNAME} 0, -365</code></li>
<li><i>Base tool page:</i> list.html?generated-part=&amp;custom-part=</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the others do not need this? I mean the ones calling comparator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It has a different implementation here, it takes the base page verbatim and puts it into the url, with the comparator links, this url is hardcoded. If you wish, I can do a hardcoded dropdown list instead of this with all the possible tool webpages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate a bit more? I think I do not follow. On comaprator urls eg http://...com:9090/comp.html?generated-part=&custom-part=--compare-traces+--only-vo... There is also custom part.

I would like tohave this behavior unifed, and I would most likely like to not hardocde anything. So enforcing to use whole diff/list/.../comp.html?generated-part=&custom-part= everywhere. Those indeed may be used in some editable drop down comobox, but user must have freedom to redirect it to any custom service. wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thats almost how it works right now, you need to write the whole thing, eg. list.html?generated-part=&custom-part= into the field and then the arguments in the next field will be directly encoded after that (that was, as you said, to allow custom services).

My previous comment was to say that the comparator links (not touched in this PR) are automatically hardoded and the custom arguments are put after the hardcoded comp.html?generated-part=&custom-part= (I found that putting the arguments from the field after the generated-part was not working properly, but putting them after custom-part was).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'ms till not follwoing why comparator should be handled differently? Please unify those two... and add the dropdown box with most common usages (the four services with generated/custom parts)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, i will do that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

but since i will be updating the class with the links, it will probably drop the current links again

Copy link
Contributor

Choose a reason for hiding this comment

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

one more reason to have them "backjuped" as examples O:)

But no, they should not get completly lost. Thy will stop working, but the fields itself shold persist. As I see it, you will be changing just one field, and then way how to combine them together, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I will be changing just one field

<li><i>Spliterator:</i> -</li>
<li><i>Tool arguments:</i> <code>-view=all-tests, -output=html, -fill, -track=%{TESTNAME} %{JOBNAME} 0, -365</code></li>
<li><i>Base tool page:</i> list.html?generated-part=&amp;custom-part=</li>
<li><i>Spliterator:</i> [.-]</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

This do not seem right. "." is everything. So maybe "\." ?

Copy link
Contributor

@judovana judovana Sep 3, 2024

Choose a reason for hiding this comment

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

or even [\\.-] ????

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Afaik, when the symbols are inside of brackets, it takes the symbol as is, so the dot is really a dot, not everything. It at least works with the comparator links

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm.. So what do you think [\.-] did? I guess [\\.-] isimply changed to three chars of \ . and -. But with [\.-] I'm not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Afaik, when the symbols are inside of brackets, it takes the symbol as is, so the dot is really a dot

Gosh. I really did not know that. I did few checks and you are right. Where did you found that written?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I found out some time on some random website when doing a school project :D

However, I always check by regexes with https://regex101.com/

Copy link
Contributor

Choose a reason for hiding this comment

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

yah... I have not found it in spec... But it seems it is indeed how it works.. thanx again!

@judovana
Copy link
Contributor

Is anything happening in this PR?

@patrikcerbak
Copy link
Collaborator Author

Yes, I'm on a last (late) summer pto this week, but I will continue next week with it :)

@patrikcerbak
Copy link
Collaborator Author

The custom links on report page can now go to any tool. There is a combobox with presets, but also a field for custom endpoint

@patrikcerbak patrikcerbak force-pushed the fixed-failed-tests-links-help branch from 1d8d964 to af4105e Compare September 23, 2024 14:11
@patrikcerbak
Copy link
Collaborator Author

rebased changes from master - the scan passes now

@patrikcerbak
Copy link
Collaborator Author

I tested it on my machine and the filled fields should remain after the update - the only thing that will need to be set is the "Preset tools" combobox

@judovana judovana merged commit fce8ac8 into jenkinsci:master Sep 26, 2024
16 checks passed
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.

2 participants