Proposal: Add initial telemetry metrics feature (backend endpoints, JS, and HTML template)#4042
Proposal: Add initial telemetry metrics feature (backend endpoints, JS, and HTML template)#4042ldelreguero wants to merge 2 commits into
Conversation
…S, and HTML template)
for more information, see https://pre-commit.ci
atodorov
left a comment
atodorov
left a comment
There was a problem hiding this comment.
Sorry for taking so long to review. Here are some initial set of comments.
|
|
||
| import { pageTelemetryReadyHandler } from '../../telemetry/static/telemetry/js/index' | ||
|
|
||
| import { drawTable as pageTelemetryMetricsReadyHandler } from '../../telemetry/static/telemetry/js/testing/metrics' |
There was a problem hiding this comment.
Please rename the original function instead of the import, see line above.
| if ((result.error.code === 401 || result.error.code === 403) && document.body.id === 'login') { | ||
| // do nothing | ||
| } | ||
| // Optionally handle error here (alert removed) |
There was a problem hiding this comment.
Why do we need this here, please explain ?
Similar comment for lines 35-38 below ?
Doesn't appear to be necessary unless you can explain what kind of problem are you trying to solve.
| from tcms.testruns.models import TestExecution, TestExecutionStatus | ||
| from tcms.testcases.models import TestCase, TestCasePlan | ||
| from tcms.testplans.models import TestPlan | ||
| from tcms.testruns.models import TestExecution, TestExecutionStatus, TestRun |
There was a problem hiding this comment.
Please see reported issue about unused import. Run pylint/isort/black lcoally too.
Formatting can be improved.
| .dataTables_paginate .ellipsis { | ||
| padding: 4px 8px; | ||
| color: #888; | ||
| } |
There was a problem hiding this comment.
CSS overrides must be in a separate .css file. If that's supposed to be used globally then add them to static/style/. Otherwise make a static file which would be included in the HTML template only for this page.
|
|
||
| {% block extra_js %} | ||
| <!-- Bootstrap CSS --> | ||
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap@3.4.1/dist/css/bootstrap.min.css"> |
There was a problem hiding this comment.
None of these are acceptable.
- You should not be including static files which are already included in base.html
- All linked files must have their src specified by
{% static %}(as in line 97 below) - Should use dependencies and versions which are already in package.json, not reinvent the wheel
- If there are missing dependencies they should be specified in package.json
| } else { | ||
| $('#metrics-help-block-server').hide(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Default security setup blocks inline scripts from executing on first page load.
- Move this into metrics.js
- Please test your code before pushing it to GitHub.
atodorov
left a comment
atodorov
left a comment
There was a problem hiding this comment.
Please rebase on top of latest code since it's been a while.
atodorov
left a comment
atodorov
left a comment
There was a problem hiding this comment.
This is what this proposed feature looks like:
maybe I am missing something due to lack of test data, however it looks very similar to the already existing feature:
Question:
What is the intended use-case for this new telemetry page ? I need to figure out what the overlap with existing implementation is b/c it looks suspiciously close and I think there is a better way to implement this.
Alexander, At the time I created issue #4038 and later tried — with many errors along the way — to generate this PR, I was attempting to explore some metrics. Test Execution Coverage = (Total number of executed test cases or scripts / Total number of test cases or scripts planned to be executed) x 100 Passed Test Cases Coverage = (Number of passed tests / Total number of tests executed) x 100 Failed Test Case Coverage = (Number of failed tests / Total number of test cases executed) x 100 Unexecuted cases = (Unexecuted tests / Total cases of test run) × 100 Test Cases Blocked = (Number of blocked tests / Total number of tests executed) x 100 Number of Test Run Per Time Period= (Number of test run / Total time) Quality Ratio = (Successful Tests Cases / Total Number of Tests Cases) x 100 Schedule Variance = [(Actual efforts – estimated efforts) / Estimated Efforts)] x 100 Although it is completely true that this information could already be visualized from the test run itself, at that time I thought it could also be displayed as standalone metrics. My context has changed, and I no longer consider these metrics useful. Please reject the PR, and I also propose closing issue 4038. |
3 participants
This PR introduces a proposal for a new telemetry metrics feature.
It includes:
Backend endpoints for telemetry metrics
JavaScript files for metrics handling and testing
An initial HTML template for displaying metrics
The goal is to start a discussion and gather feedback on the approach and implementation.
Please note:
This contribution was created entirely with the help of AI, so I expect there will be a lot of work needed to polish and improve the code. I apologize in advance for any rough edges and appreciate your understanding and feedback!