feat(tests): improving E2E HTML report and output on remediation tests

[cx-ricardo-jesus]

Reason for Proposed Changes

• Currently, when the remediation tests fail, there is no indication of which files were actually remediated.
• This information could help in situations where the remediation tests fail, because it will be easier to see which files were remediated, decreasing the time spent analysing which scenarios are causing these tests to fail.
• The HTML reported generated when an E2E test fails, more specifically when there is an actual results vs expected result. In this scenario, a long continuous block of unorganized information regarding.

Proposed Changes

• On the Summary struct present inside the utils.go file, added the RemediatedFiles, which is an array of strings, meant to store the path of the files that were remediated.
• Inside the method testRemediationQuery, added a new map, meant to store the connections between the temporary files used to do the remediations and the original files from which those temporary files came. The key is the path to these temporary files, and the value is the path to the original file.
• The new map tempToOriginal will be necessary to send the path to the original file through the RemediateFile method inside the summary, which now takes one more argument (original_file_name). Within this RemediateFile method, this path is passed to the writeRemediation function to append the original filename to the new field RemediatedFiles in the Summary struct.
• Returning to the method mentioned earlier, testRemediationQuery, the test now returns a different message from the original, and now also mentions the files that were remediated.
• Other changes can be seen in the remediation_test.go and remediate.go, because both use the changed RemediateFile method that now takes one more argument.
• On the remediation_test.go, I passed the filePathCopyFrom, which is the path of the original file used for the tests.
• On the remediate.go, I passed an empty string, because this file handles the remediation command, and in this command, which is meant to apply a remediation to a certain file, the original file path was already sent to the RemediateFile method.

---

• Regarding the changes on the E2E tests, I basically made changes on the following two files: e2e/utils/json.go and .github/scripts/report/main.go. The first one writes the results of the E2E tests to a JSON file, and the second is a script that transforms the information in those JSON files and produces an HTML report.

• On e2e/utils/json.go, I basically added three new helper functions: func formatPayload(payload []string), func formatVulnFiles(files []map[string]interface{}) and func toComparableFiles(queries []model.QueryResult).

• The helper function formatPayload formats a slice of strings into a single readable string, with each line separated by a newline. Used to improve the readability of the failure message in FileCheck when the number of lines between the expected and actual output files does not match. The script will use this improved output to generate a better HTML report.

• The helper function toComparableFiles(queries []model.QueryResult) flattens a list of QueryResult, into a list of of maps, each one representing a VulnerableFile(the result from a scan in a query) enriched with it's parent QueryName so, now, the other helper function formatVulnFiles can take each result and pass the name of the query associated with that result.

• Regarding the changes made in the script .github/scripts/report/main.go I added on the TestsData struct, the field ExpectedActual that is of type ExpectedActual. That ExpectedActual struct has the following fields:

  • ExtraElements, which contained a list of the lines from the Expected Result and the Actual Result, that is the result of the extra elements that are given by the require.ElementsMatch inside the json.go file.

  • TestInfo, that is the information about which test is returning the error as shown below:

    

  • Messages, that is, the output returned from the Messages in the require package functions used in the json.go file. This also stores the positive and expected results.

  • The FailOutput is the output in the test that starts with the --- FAIL as shown below:

    

  • Both of the fields ExtraElements and Messages have the type ActualExpectedWithStatus that have a list of CodeLineStatus that is basically a line of the output with its status. The status will be used to parse the lines that differ from the expected result and the actual result to allow them to be highlighted in the generated HTML report.

• In this script, I added five helper functions:

  • cleanOutput(s string) string cleans a line of test output by removing noise added by the go-spew library, such as type annotations (e.g. (string), (int), (model.IssueType)) and length annotations (e.g., (len=3)). This ensures that when lines are compared and displayed in the report, they are readable and not cluttered with internal Go type information.
  • extractPayloadDiffLines(failLog []string) ExpectedActual parses the fail log output for tests that failed due to a mismatch in the number of lines between the expected and actual payload files. It extracts the expectedPayload and actualPayload sections into the Messages field, and the --- FAIL line into FailOutput, by iterating over the log lines and switching state based on known prefixes.
  • extractExpectedActualLines(failLog []string) ExpectedActual parses the fail log output for tests that failed due to mismatched query results (i.e. when require.ElementsMatch fails). It extracts the extra elements reported in "list A" and "list B" into the ExtraElements field, the full expected and actual query content into Messages, and the --- FAIL line into FailOutput. Each line is cleaned using cleanOutput before being stored.
  • isDifferentNumberOfLines(failLog []string) bool inspects the fail log to determine whether the test failed because the expected and actual output files have a different number of lines. It does so by checking if the log contains both "Expected file number of lines:" and "Actual file number of lines:" markers, which are produced by the FileCheck function in json.go.
  • isExpectedVsActual(failLog []string) bool inspects the fail log to determine whether the test failed due to a query result mismatch reported by require.ElementsMatch. It checks for the presence of both "extra elements in list A:" and "extra elements in list B:" markers in the log, which are the markers used by the testify library when two slices do not match.
  • compareMessageContent(expectedActual *ExpectedActual) iterates over the Messages and ExtraElements fields of an ExpectedActual and compares each line at the same position between the expected and actual content. If a line differs, or if one side has more lines than the other, the differing lines are marked with Status = true. This status is later used by the HTML report template to highlight the lines that differ between the expected and actual output.

---

• To reproduce the case when there are extra elements and a comparison between an expected result and the actual result, it is only necessary to change, for example, the line in the E2E_CLI_068_RESULT.json in the e2e/fixtures file.
• To reproduce the scenario when there is only a different number of lines between the expected result and the actual result, a result can be deleted in a file from the e2e/fixtures directory (e.g., E2E_CLI_032.json).

---

• As shown below, the HTML report is clearly more readable and shows the source of the error more clearly.

• Before:

  

• After:
  

  

I submit this contribution under the Apache-2.0 license.

[github-actions]

KICS version: v2.1.18

Category Results CRITICAL 0 HIGH 0 MEDIUM 0 LOW 0 INFO 0 TRACE 0 TOTAL 0	Metric Values Files scanned 1 Files parsed 1 Files failed to scan 0 Total executed queries 47 Queries failed to execute 0 Execution time 0

[cx-artur-ribeiro]

The .html and .js seems all good to me, but I have some questions regarding code maintainence and duplication. Some notes are just quality of life improvements.

Can you check and give me your opinion please?