filter for test coverage

[Prahitha]

note: the filters will temporarily be at the end of the table -> $(column.footer()).empty()
it is not fully implemented yet. the column.draw() inside the conditional statements doesn't work and needs some work.

[Prahitha]

Note: Still a WIP
Here is a small demo of what it does, there are a few bugs (like the one I paused at in the middle of the video)
https://user-images.githubusercontent.com/44160152/120211208-4cffc400-c24e-11eb-8011-6da912c8f8da.mp4

(the select dropdown was not recorded in the screen recording)
For the first column, I selected all (i.e., all test cases passed) and then selected 0 for the next column. This basically filters from the result of the previous one, if not cleared. To clear an active filter, the empty select option has to be selected.

[tgorochowik]

LGTM in general, nice work!

I am still not sure about putting this in the cell it is now, it seems to limit the visibility of the total number of tests passed?

also: what happens if you select something for multiple cells? is this a logical and? or a logical or? it is not immediately clear I think

[tgorochowik]

what exactly does this do? do we have to have that list literal here?

[Prahitha]

so I listed them out to not have the dropdown for the tags (first 2 columns) but I'll see if this can be done in some other way

[hzeller]

So this will have to be changed whenever we add a new tool ? This sounds brittle. Can this be formulated as 'everything but the first two columns' instead ?

[Prahitha]

I believe this is resolved in the latest commit. I'll also look into adding a separate JS file for the script. Thanks for the suggestions!

[tgorochowik]

CC @hzeller

[Prahitha]

thanks! so the position of the drop-downs definitely has to change and I planned on working on it once the functionality seemed okay. and, I believe it's a logical and because it filters out from the previous result only. i'll look into other ways if it's not as intuitive, please let me know

[hzeller]

So this will have to be changed whenever we add a new tool ? This sounds brittle. Can this be formulated as 'everything but the first two columns' instead ?

[hzeller]

General question to consider: instead of having in-line implementation of functions, putting them in a separate JS file might be easier to maintain in the long term.
It is easy to accumulate important code that is hard to understand if buried in a large HTML template (not sure what are good practices are to break this up, as it seems that the closure takes a few values from the context (such as $(this).val()) - so possibly the function to be implemented externally needs explicit parameters; but that is good for documentation).

So while this looks not a large problem here at this point (and maybe the conclusion might be that this particular use might be benign), it might become some problem in the future. So worthwhile considering a general plan for code structuring.

[Prahitha]

below is a demo of how the toggle looks and works. also, just to clarify, the filtering is currently logical and but can support logical or too. any suggestions on how to implement it differently are welcome. thanks!

toggleFilter.mp4

[github-actions]

Compared test results

tool	new_failures	new_passes	added	removed	not_affected
Slang	0	7	0	0	4861
YosysSv	0	0	0	0	4868
Icarus	0	0	0	0	4523
Verilator	0	339	0	0	4529
Verible	0	0	0	0	4523
Yosys	0	1	0	0	4867
moore	0	0	0	0	4623
Sv2v_zachjs	1	0	0	0	4622
UhdmYosys	2	2	0	0	4519
Surelog	0	0	0	0	4868
moore_parse	0	0	0	0	4623
tree_sitter_verilog	0	0	0	0	4523
Odin	0	0	0	0	4623
VeribleExtractor	0	0	0	0	4523
UhdmVerilator	0	0	0	0	4868
sv_parser	0	0	0	0	4623

[tgorochowik]

Adding it this way will not allow us to easily mix the filter options with other filter types (once we add them).

"filter type" should be a separate field, so once we click "show filter options" we should get a combo box with "filter type" (for now we should be able to only select "coverage" but other filter types should be possible to select in the future.

Then once you do select "coverage" another two inputs should appear - one for the tool selection and another one for coverage selection.

below all of that we should have two buttons - "apply" and "add"

"apply" is self-explanatory (and can be optional - it is okay to automatically apply everything once the row i actually correct - I think this is done like this at the moment)
"add" should generate another row of fields which we can use to combine the current filter (the first row) with another filter (test coverage for another tool for now, other filter types in the future).

Is that explanation clear? Or do you need more details?

Another option - a possibly more modern approach - would be to make the search bar recognize some keywords and make it apply the filters based on what is typed into the search bar - something similar to what github and gitlab do: https://docs.gitlab.com/ee/user/search/ (for a start we could use the google-like filter_type:filter_input notation) - but I think this may be harder to implement

@hzeller what do you think?

[Prahitha]

Thanks for the detailed explanation! I've attached an image below to confirm if I'm understanding properly. Please let me know, thanks

[tgorochowik]

Hmm, not exactly, please see the attached diagram, the steps are split to separate drawings, hopefully it will be easier to understand what I mean