Skip to content
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

Separate automatic from manual checks #61

Closed
MarcoMinghini opened this issue Feb 7, 2019 · 12 comments
Closed

Separate automatic from manual checks #61

MarcoMinghini opened this issue Feb 7, 2019 · 12 comments
Labels
? Unknown status or information missing in the EIP (temp. label) EIP-approved EIP approved by the Steering Group

Comments

@MarcoMinghini
Copy link
Member

MarcoMinghini commented Feb 7, 2019

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 #60).

Funding

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

Funding is ensured by the JRC.

@MarcoMinghini MarcoMinghini added the EIP-draft Announcement. Will not be discussed further as long as the necessary information is available. label Feb 7, 2019
@MarcoMinghini MarcoMinghini changed the title Separate automatic from manual checs [DRAFT] Separate automatic from manual checks [DRAFT] Feb 7, 2019
@michellutz michellutz added EIP Improvement Proposal. Put up for discussion. and removed EIP-draft Announcement. Will not be discussed further as long as the necessary information is available. labels Feb 8, 2019
@MarcoMinghini MarcoMinghini changed the title Separate automatic from manual checks [DRAFT] Separate automatic from manual checks Feb 28, 2019
@carlospzurita
Copy link

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
Copy link

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:

  • Core (etf-bsxds)

@cportele
Copy link
Contributor

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
Copy link

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
Copy link

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
Copy link

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
Copy link
Contributor

jonherrmann commented May 6, 2019

  1. ) Clemens has shown an example of the parameters: Improve filtering of test reports #32 (comment) . 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
Copy link
Contributor

cportele commented May 7, 2019

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
Copy link

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
Copy link
Member Author

MarcoMinghini commented Jun 17, 2019

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
Copy link

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.

@michellutz michellutz added EIP-approved EIP approved by the Steering Group and removed EIP Improvement Proposal. Put up for discussion. labels Jul 18, 2019
@jonherrmann jonherrmann added the ? Unknown status or information missing in the EIP (temp. label) label May 23, 2022
@MarcoMinghini
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
? Unknown status or information missing in the EIP (temp. label) EIP-approved EIP approved by the Steering Group
Projects
None yet
Development

No branches or pull requests

5 participants