-
Notifications
You must be signed in to change notification settings - Fork 10
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
improvement(frontend): Add frontend endpoint for plugin/run_id #478
Conversation
This commit adds a new endpoint which replaces hard to use /test/test_id/runs?additionalRuns[]=run_id endpoint into a much more simpler /tests/plugin_name/run_id endpoint, allowing to build a link to the test run without knowing test_id in advance and simplifying sharing/searching for a run. Part of Task: scylladb/qa-tasks#1490
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if two runs are selected?
Besides, I can't wait to see it merged :)
LGTM
In both cases it wasn't possible to directly link to two runs in frontend. While the old endpoint supports array declaration To elaborate, the link exists on each run selected in its title, e.g. |
@@ -133,7 +133,7 @@ | |||
<div class="d-flex px-2 py-2 mb-1 border-bottom bg-white "> | |||
<div class="p-1"> | |||
{#if testRun} | |||
<a class="link-dark" href="/test/{testRun.test_id}/runs?additionalRuns[]={testRun.id}"> | |||
<a class="link-dark" href="/tests/{testInfo.test.plugin_name}/{testRun.id}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the /{testInfo.test.plugin_name}
part is needed?
I guess it will be much more useful to have release name than just testing tool name.
For example:
tests/releases/enterprise-2024.2/%test-id%
says clearly about the belonging of a test run to a release.
So it will be easy to distinguish it from the following:
tests/staging/valerii/%test-id%
.
Which I personally find useful - we get info not even opening the link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the
/{testInfo.test.plugin_name}
part is needed? I guess it will be much more useful to have release name than just testing tool name.For example:
tests/releases/enterprise-2024.2/%test-id%
says clearly about the belonging of a test run to a release.So it will be easy to distinguish it from the following:
tests/staging/valerii/%test-id%
.Which I personally find useful - we get info not even opening the link.
Testing tool is needed (or test-id, as it was previously) to determine which result table we query for those results, so it would have to stay. The release marker can be introduced, as an alternative URL, but release name is not something which can be guessed from testing frameworks themselves, aside from making them query the run (and I want to avoid that to make URL as simple to generate as possible for a testing framework.)
For argus, we can add another route which would act similarly to this one, like
/release/scylla-master/tests/scylla-cluster-tests/$id
or
/tests/scylla-cluster-tests/scylla-master/folder-name/test-name/$id
This one can be the one you'll get from clicking on the title bar and the URLs in the mail can provide a 301 redirect to this URL once you click them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the use case of getting from emails or elasticsearch data into Argus run
What's in this PR fits best, introducing more data into the url, complicates how we can build it where needed.
As suggested we can have more options for better sharing out of Argus, but we still need a way to get from test-id to Argus page
This commit adds a new endpoint which replaces hard to use
/test/test_id/runs?additionalRuns[]=run_id endpoint into a much more
simpler /tests/plugin_name/run_id endpoint, allowing to build a link to
the test run without knowing test_id in advance and simplifying
sharing/searching for a run.
Part of Task: scylladb/qa-tasks#1490