Skip to content

Commit

Permalink
fix(Test Starter Configs): Fix false-positive findings (#409)
Browse files Browse the repository at this point in the history
This fixes a false-positive finding of the property `theme` inside
[Configuration.beforeBootstrap.qunit.js](https://github.com/SAP/openui5/blob/master/src/sap.ui.core/test/sap/ui/core/qunit/bootstrap/Configuration.beforeBootstrap.qunit.js).
This resulted in undefined runtime errors.
`theme` should only be checked for Test Starter configs. 

Changes:
* Now, the **file name** is checked against a Regex and optional
chaining was optimized to prevent these runtime errors further.

* A sample `Configuration.beforeBootstrap.qunit.js` was added as a test
artifact. As seen in the snapshots, there are no more findings for this
file.

* In the course of adding a Regex check for the file name,
`testsuite2.qunit.js` was renamed to
`testsuite.special-case-quotes.qunit.js` to be aligned with framework
functionality.
  • Loading branch information
maxreichmann authored Nov 15, 2024
1 parent ddc6fb1 commit cf166f7
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 12 deletions.
23 changes: 14 additions & 9 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1308,21 +1308,26 @@ export default class SourceFileLinter {
}

analyzeTestsuiteThemeProperty(node: ts.PropertyAssignment) {
// Check if the node is part of a testsuite config file by its file name.
// This is the same check as in the framework and prevents false-positives
// https://github.com/SAP/openui5/blob/32c21c33d9dc29a32bf7ee7f41d7bae23dcf086b/src/sap.ui.core/src/sap/ui/test/starter/_utils.js#L287
const validTestSuiteName = /^\/testsuite(?:\.[a-z][a-z0-9-]*)*\.qunit\.(?:js|ts)$/;
if (!validTestSuiteName.test(node.getSourceFile().fileName)) return;

// In a Test Starter testsuite file,
// themes can be defined as default (1.) or for test configs individually (2.).
// The schema for these files can be found here (under 'The UI5 Test Suite Module'):
// https://ui5.sap.com/#/topic/22f50c0f0b104bf3ba84620880793d3f

// (1.) and (2.) are checks for these two possible structures,
// which use surrounding property names to determine the context.

// We cannot use the best practice file name "testsuite.qunit.js/ts",
// to determine if a file is a Test Starter testsuite file,
// because the file name can be arbitrary.
// Therefore, we need checks (1.) and (2.) and set a flag to true afterwards.
// which use surrounding property names to determine the context
// and set the flag 'isTestStarterStructure' to true afterwards.
let isTestStarterStructure = false;

const oneLayerUp = node.parent.parent;
const twoLayersUp = oneLayerUp?.parent.parent;
const threeLayersUp = twoLayersUp?.parent.parent;
const oneLayerUp = node.parent?.parent;
const twoLayersUp = oneLayerUp?.parent?.parent;
const threeLayersUp = twoLayersUp?.parent?.parent;

// Check if "theme" property is inside "ui5: {...}" object
if (oneLayerUp && ts.isObjectLiteralElement(oneLayerUp) &&
removeQuotes(oneLayerUp.name?.getText()) === "ui5") {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// This file was a False Positive for the new Test Starter detections.

window["sap-ui-config"] = {
theme: "SapSampleTheme2", // This should not get detected by the Test Starter detections
language: "en",
calendarType: "islamic",
logLevel: "WARNING",
noConflict: "true",
libs: "sap.ui.core"
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
sap.ui.define(function () {
"use strict";
return {
name: "QUnit test suite with deprecated themes (JS) - Property names are surrounded by quotes",
name: "QUnit test suite with deprecated themes (JS) - Special Cases: Property names are surrounded by quotes",
"defaults": {
page: "ui5://test-resources/sap/ui/demo/todo/Test.qunit.html?testsuite={suite}&test={name}",
qunit: {
Expand Down
19 changes: 17 additions & 2 deletions test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,21 @@ Generated by [AVA](https://avajs.dev).
},
]

## General: Configuration.beforeBootstrap.qunit.js

> Snapshot 1
[
{
coverageInfo: [],
errorCount: 0,
fatalErrorCount: 0,
filePath: 'Configuration.beforeBootstrap.qunit.js',
messages: [],
warningCount: 0,
},
]

## General: Control_ManagedObject.js

> Snapshot 1
Expand Down Expand Up @@ -2836,7 +2851,7 @@ Generated by [AVA](https://avajs.dev).
},
]

## General: testsuite2.qunit.js
## General: testsuite.special-case-quotes.qunit.js

> Snapshot 1
Expand All @@ -2845,7 +2860,7 @@ Generated by [AVA](https://avajs.dev).
coverageInfo: [],
errorCount: 3,
fatalErrorCount: 0,
filePath: 'testsuite2.qunit.js',
filePath: 'testsuite.special-case-quotes.qunit.js',
messages: [
{
column: 5,
Expand Down
Binary file modified test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap
Binary file not shown.

0 comments on commit cf166f7

Please sign in to comment.