etf-validator / governance

ETF Steering Group and the Technical Committee documents
1 stars 2 forks source link

Separate automatic from manual checks #61

Closed MarcoMinghini closed 2 years ago

MarcoMinghini commented 5 years ago

Background and Motivation:

Explain the problem behind this proposal. If the problem has already been discussed externally, please add a link. Source code or data snippets, diagrams, graphics that illustrate the problem are welcome.

Manual checks (i.e. those checks that cannot be done automatically, but are left to the user) are somehow "hidden" or "sparse" in the test report. When looking at a test report, the user has not a clear/immediate idea of which are the manual checks required. This was also discussed with the members of MIWP Action 2017.4 (see here).

Proposed change

Explain what should be changed in the software. Mockups, screenshots, diagrams or other graphics are welcome.

Following discussion with the members of MIWP Action 2017.4, it was agreed that it would be useful to separate the manual checks by showing them as a checklist at the bottom of the report, in order to facilitate the user's understanding.

Alternatives

Describe alternative solutions/features that have been considered.

Manual checks can be also visually recalled in the new proposed graphics to improve visualization of test reports (see https://github.com/etf-validator/governance/issues/60).

Funding

Is there full or partial funding available for implementing this proposal?

Funding is ensured by the JRC.

carlospzurita commented 5 years ago

We think that this is improvement can be achieved adding to the stylesheet in https://github.com/etf-validator/etf-bsxds/blob/next/src/main/resources/xslt/default/TestRun2DefaultReport.xsl another field in the report, collecting all the tests that are marked with the PASSED_MANUAL flag, and display them at the bottom of the report, as a list of collapsible items with the description of the test.

carlospzurita commented 5 years ago

The proposed solution would only need a visual improvement, given that all the information for the test results is available for the report templating. We just need to add another section at the end of the report template, collecting all the manual checks needed. This is aimed to ease the task for reviewers, not to add any new information on the reports.

This improvement will affect the following components:

cportele commented 5 years ago

I have concerns about this approach, if I understand it correctly, as it could make the HTML representation of the result to some extent inconsistent with the XML/JSON representation. And also confusing as a test case with 7 test assertions where two have a PASSED_MANUAL result would state that there are 7 assertions, but only 5 are shown in the test case (as the other two are somewhere at the end of the report, outside of the test case context). To make it work, we should have a clear idea how to avoid such issues.

carlospzurita commented 5 years ago

The test manual checks don't have to disappear from the HTML representation, they can be kept on the report as usual. What is proposed is to add a separated checklist of manual cheks that the reviewer must carry out in order to complete the test run. This is, of course, redundant information, but from the end user perspective it would be a neat feature in order to have all the information at glance.

From a process perspective, the workflow would be the same as usual. The only that has to change is the report transformer to HTML, adding on the stylesheet the list of required manual checks, if any.

michellutz commented 5 years ago

Considering a number of possible options to solve this issue, we would like to discuss the feasibility of the following options:

  1. Enable an option in the ManageTestRuns operation in the API allowing to skip manual tests.
  2. Enable an option in the Configure Test Run dialogue to skip manual tests (using the operation option in (1))
  3. Change the representation for passed_manual tests (and test suites) to use the same green as passed tests (and test suites) but including some icon (a hand?) to indicate that manual tests need to be performed (plus maybe extracting all manual tests into a checklist at the end of the report) -- this could also be an INSPIRE-specific customisation.
carlospzurita commented 5 years ago

The approach 3 could be represented on the HTML report visually as a separated list at the end of the report. We made a quick mockup to represent this, with all the manual checks displayed at the bottom, additionally to their own place on the XML report. mockup-manual-checks2 As for the other options, the ConfigureTestRun has the regex feature to leave out tests, so maybe could be reused to implement a chekbox to skip them altogether.

jonherrmann commented 5 years ago
  1. ) Clemens has shown an example of the parameters: https://github.com/etf-validator/governance/issues/32#issuecomment-489603205 . I propose to use the parameter name skipManualTests, which will then be reserved as a standard parameter name.

  2. ) One of the basic concepts of ETF: Parameters from the ETS are rendered in the web interface :)

cportele commented 5 years ago

As discussed in the call:

See https://github.com/inspire-eu-validation/ets-repository/blob/master/inspire-bsxets.xq (and also in inspire-md-bsxets.xq and inspire-noggeo-bsxets.xq).

Declare a variable for the parameter, for example add in row 282 something like

declare variable $skipManualTests external := "false";

Then change row 28 which currently reads

let $disabled := if (not(matches($assertion/etf:label,$tests_to_execute)) or $type/@ref='EID92f22a19-2ec2-43f0-8971-c2da3eaafcd2' (:disabled :)) then 'NOT_APPLICABLE' else if ($type/@ref='EIDb48eeaa3-6a74-414a-879c-1dc708017e11' (: manual :)) then 'PASSED_MANUAL' else ()

to something where $type/@ref='EIDb48eeaa3-6a74-414a-879c-1dc708017e11' (: manual :) and $skipManualTests = 'true' results in status NOT_APPLICABLE, too.

carlospzurita commented 5 years ago

We can include the parameter to skip the manual tests altogether on the Star Test dialog. The ETS developed should check this parameter before executing any manual test step.

To complete the proposal of a manual test checklist, we have a new mockup to try and exemplify better the original propose of @MarcoMinghini . This would only be included on the HTML report, but won't be on the original XML data of the test run.

Captura de pantalla de 2019-06-10 13-33-00

MarcoMinghini commented 5 years ago

This screenshot only shows the separate checklist of manual tests at the end of the test report; the proposal was to also show manual tests in the report using the same green color of passed tests, plus an icon (e.g. a hand) to indicate that manual tests are also required.

carlospzurita commented 5 years ago

We changed the colors and the icon to the manual tests as requested. Unfortunately, the framework used for the UI does not have a hand icon, so we put an user icon. image Due to the length of the report, is difficult to screenshot at the same time the manual checks list at the bottom and the color scheme change, so we have separated it in another image.

MarcoMinghini commented 2 years ago

With the development of the new INSPIRE UI, this proposal is no longer relevant for us. Unless there are different views, we suggest to close it.