fix(tests-table): filter by individual table path #882
Conversation
murilx
force-pushed
the
fix/tests-table-path-filter
branch
from
7891cd9 to
25a2a38
Compare
| @@ -56,6 +56,44 @@ export interface ITestsTable { | |||
| currentPathFilter?: string; | |||
| } | |||
|
|
|||
| type StatusGroup = Pick< | |||
There was a problem hiding this comment.
change to TestStatusGroup, to differ of StatusGroups (build)
There was a problem hiding this comment.
Changed to TPathTestsStatus to make it very clear from where these fields are coming from!
| return result; | ||
| }), | ||
| individual_tests: test.individual_tests.filter( | ||
| t => t.status?.toUpperCase() === StatusTable.FAIL, |
There was a problem hiding this comment.
This toUpperCase is really needed? If yes, in the statusCounter function you may transform the status string to upper case too
There was a problem hiding this comment.
That's true, if toUpperCase was needed, it should be included in statusCounter as well. Seeing that statusCounter is working I'd have to think that toUpperCase is not necessary in any situation. I'll change this and check to see if it's working correctly
There was a problem hiding this comment.
I think it would be very unlikely that the database changes the way the status are written (from upper case to lower case for example), but since this does not cause any problems (like performance issues), I'd still say we keep this and add the same to statusCounter just to be sure
murilx
force-pushed
the
fix/tests-table-path-filter
branch
from
25a2a38 to
0955fc6
Compare
|
this is a better test subject than the one in the link, since there are tests paths that are different from the group path |
| }; | ||
| const individualTest = test.individual_tests.filter(t => { | ||
| let dataIncludesPath = true; | ||
| if (isValidPath) dataIncludesPath = t.path.includes(path); |
There was a problem hiding this comment.
nit:
| if (isValidPath) dataIncludesPath = t.path.includes(path); | |
| if (isValidPath) { | |
| dataIncludesPath = t.path.includes(path); | |
| } |
we should probably add this to eslint
There was a problem hiding this comment.
All add it to another PR just to not "add noise" to this one
| if (isValidPath) dataIncludesPath = t.path.includes(path); | ||
| if (dataIncludesPath) { | ||
| statusCounter(localGroup, t.status); | ||
| statusCounter(globalGroup, t.status); |
There was a problem hiding this comment.
I think this is missplaced, the filter is affecting these counts just like the global group
There was a problem hiding this comment.
I believe this is the expected behavior based on what we currently see in tree and hardware details. globalGroup would be the sum of all localGroup. If all tests are filtered out, then globalGroup will correctly be 0-0-0-0
Filter by the path of every individual test when `updatePathFilter` is undefined Closes #875
murilx
force-pushed
the
fix/tests-table-path-filter
branch
from
0955fc6 to
a772382
Compare
Filter by the path of every individual test when
updatePathFilteris undefinedCloses #875
How to test
updatePathFilterthere shouldn't be any differences from the version on staging. However, they are still influenced by this implementation since all of the status counting is done in the filteredDatauseMemo.