Skip to content

Commit

Permalink
feat: Detect non-Test Starter test setups (#448)
Browse files Browse the repository at this point in the history
JIRA: CPOUI5FOUNDATION-894
  • Loading branch information
d3xter666 authored Jan 8, 2025
1 parent 111ce1f commit 8d7442f
Show file tree
Hide file tree
Showing 13 changed files with 600 additions and 47 deletions.
1 change: 1 addition & 0 deletions src/linter/LinterContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export interface PositionRange {
export interface LintMetadata {
// The metadata holds information to be shared across linters
directives: Set<Directive>;
transformedImports: Map<string, Set<string>>;
}

export default class LinterContext {
Expand Down
22 changes: 22 additions & 0 deletions src/linter/html/transpiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export default async function transpileHtml(
lintBootstrapAttributes(bootstrapTag, report);
}

detectTestStarter(resourcePath, scriptTags, context);

scriptTags.forEach((tag) => {
// Tags with src attribute do not parse and run inline code
const hasSrc = tag.attributes.some((attr) => {
Expand Down Expand Up @@ -52,6 +54,26 @@ export default async function transpileHtml(
}
}

function detectTestStarter(resourcePath: ResourcePath, scriptTags: Tag[], context: LinterContext) {
const shouldBeMigrated = scriptTags.some((tag) => {
const isTestsuiteQunitFile = /testsuite(?:\.[a-z][a-z0-9-]*)*\.qunit\.html$/.test(resourcePath);
return (isTestsuiteQunitFile && !tag.attributes.some((attr) => {
return attr.name.value.toLowerCase() === "src" &&
(attr.value.value.endsWith("/resources/sap/ui/test/starter/createSuite.js") ||
attr.value.value === "resources/sap/ui/test/starter/createSuite.js");
})) ||
(!isTestsuiteQunitFile && resourcePath.endsWith("qunit.html") && !tag.attributes.some((attr) => {
return attr.name.value.toLowerCase() === "src" &&
(attr.value.value.endsWith("/resources/sap/ui/test/starter/runTest.js") ||
attr.value.value === "resources/sap/ui/test/starter/runTest.js");
}));
});

if (shouldBeMigrated) {
context.addLintingMessage(resourcePath, MESSAGE.PREFER_TEST_STARTER, undefined as never);
}
}

function findBootstrapTag(tags: Tag[]): Tag | undefined {
// First search for script tag with id "sap-ui-bootstrap"
for (const tag of tags) {
Expand Down
11 changes: 11 additions & 0 deletions src/linter/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const RULES = {
"no-pseudo-modules": "no-pseudo-modules",
"parsing-error": "parsing-error",
"ui5-class-declaration": "ui5-class-declaration",
"prefer-test-starter": "prefer-test-starter",
} as const;

export enum LintMessageSeverity {
Expand Down Expand Up @@ -63,6 +64,7 @@ export enum MESSAGE {
PARTIALLY_DEPRECATED_ODATA_MODEL_V2_CREATE_ENTRY,
PARTIALLY_DEPRECATED_ODATA_MODEL_V2_CREATE_ENTRY_PROPERTIES_ARRAY,
PARTIALLY_DEPRECATED_PARAMETERS_GET,
PREFER_TEST_STARTER,
REDUNDANT_BOOTSTRAP_PARAM,
REDUNDANT_BOOTSTRAP_PARAM_ERROR,
REDUNDANT_VIEW_CONFIG_PROPERTY,
Expand Down Expand Up @@ -550,6 +552,15 @@ export const MESSAGE_INFO = {
},
},

[MESSAGE.PREFER_TEST_STARTER]: {
severity: LintMessageSeverity.Warning,
ruleId: RULES["prefer-test-starter"],

message: () => "To save boilerplate code and ensure compliance with UI5 2.x best practices," +
" please migrate to the Test Starter concept",
details: () => "{@link topic:032be2cb2e1d4115af20862673bedcdb Test Starter}",
},

[MESSAGE.REPLACED_BOOTSTRAP_PARAM]: {
severity: LintMessageSeverity.Error,
ruleId: RULES["no-deprecated-api"],
Expand Down
42 changes: 37 additions & 5 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ import {findDirectives} from "./directives.js";

const log = getLogger("linter:ui5Types:SourceFileLinter");

// 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 VALID_TESTSUITE = /^\/testsuite(?:\.[a-z][a-z0-9-]*)*\.qunit\.(?:js|ts)$/;

interface DeprecationInfo {
symbol: ts.Symbol;
messageDetails: string;
Expand Down Expand Up @@ -58,6 +62,7 @@ export default class SourceFileLinter {
#boundVisitNode: (node: ts.Node) => void;
#fileName: string;
#isComponent: boolean;
#hasTestStarterFindings: boolean;

constructor(
private context: LinterContext,
Expand All @@ -76,6 +81,7 @@ export default class SourceFileLinter {
this.#boundVisitNode = this.visitNode.bind(this);
this.#fileName = path.basename(resourcePath);
this.#isComponent = this.#fileName === "Component.js" || this.#fileName === "Component.ts";
this.#hasTestStarterFindings = false;
}

// eslint-disable-next-line @typescript-eslint/require-await
Expand All @@ -88,6 +94,12 @@ export default class SourceFileLinter {
findDirectives(this.sourceFile, metadata);
}
this.visitNode(this.sourceFile);

if (this.sourceFile.fileName.endsWith(".qunit.js") && // TS files do not have sap.ui.define
!metadata?.transformedImports?.get("sap.ui.define")) {
this.#reportTestStarter(this.sourceFile);
}

this.#reporter.deduplicateMessages();
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
Expand Down Expand Up @@ -642,6 +654,13 @@ export default class SourceFileLinter {
}

analyzeNewExpression(node: ts.NewExpression) {
if (/\.qunit\.(js|ts)$/.test(this.sourceFile.fileName) &&
((ts.isPropertyAccessExpression(node.expression) && node.expression.name.getText() === "jsUnitTestSuite") ||
(ts.isIdentifier(node.expression) && node.expression.getText() === "jsUnitTestSuite")
)) {
this.#reportTestStarter(node);
}

const nodeType = this.checker.getTypeAtLocation(node); // checker.getContextualType(node);
if (!nodeType.symbol || !this.isSymbolOfUi5OrThirdPartyType(nodeType.symbol)) {
return;
Expand Down Expand Up @@ -790,6 +809,9 @@ export default class SourceFileLinter {
this.#analyzeMobileInit(node);
} else if (symbolName === "setTheme" && moduleName === "sap/ui/core/Theming") {
this.#analyzeThemingSetTheme(node);
} else if (/\.qunit\.(js|ts)$/.test(this.sourceFile.fileName) &&
symbolName === "ready" && moduleName === "sap/ui/core/Core") {
this.#reportTestStarter(node);
}
}

Expand Down Expand Up @@ -826,11 +848,17 @@ export default class SourceFileLinter {
}
}

const propName = getPropertyName(reportNode);

this.#reporter.addMessage(MESSAGE.DEPRECATED_FUNCTION_CALL, {
functionName: getPropertyName(reportNode),
functionName: propName,
additionalMessage,
details: deprecationInfo.messageDetails,
}, reportNode);

if (propName === "attachInit" && /\.qunit\.(js|ts)$/.test(this.sourceFile.fileName)) {
this.#reportTestStarter(reportNode);
}
}

getSymbolModuleDeclaration(symbol: ts.Symbol) {
Expand Down Expand Up @@ -915,6 +943,13 @@ export default class SourceFileLinter {
});
}

#reportTestStarter(node: ts.Node) {
if (!this.#hasTestStarterFindings) {
this.#reporter.addMessage(MESSAGE.PREFER_TEST_STARTER, node);
this.#hasTestStarterFindings = true;
}
}

#analyzeParametersGetCall(node: ts.CallExpression) {
if (node.arguments.length && ts.isObjectLiteralExpression(node.arguments[0])) {
// Non-deprecated usage
Expand Down Expand Up @@ -1299,10 +1334,7 @@ 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;
if (!VALID_TESTSUITE.test(node.getSourceFile().fileName)) return;

// In a Test Starter testsuite file,
// themes can be defined as default (1.) or for test configs individually (2.).
Expand Down
23 changes: 22 additions & 1 deletion src/linter/ui5Types/amdTranspiler/tsTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import replaceNodeInParent, {NodeReplacement} from "./replaceNodeInParent.js";
import {toPosStr, UnsupportedModuleError} from "./util.js";
import rewriteExtendCall, {UnsupportedExtendCall} from "./rewriteExtendCall.js";
import insertNodesInParent from "./insertNodesInParent.js";
import LinterContext from "../../LinterContext.js";
import LinterContext, {LintMetadata} from "../../LinterContext.js";
import {findDirectives} from "../directives.js";

const log = getLogger("linter:ui5Types:amdTranspiler:TsTransformer");
Expand Down Expand Up @@ -113,6 +113,13 @@ function transform(
const moduleDeclaration = parseModuleDeclaration(node.arguments, checker);
const moduleDefinition = moduleDeclarationToDefinition(moduleDeclaration, sourceFile, nodeFactory);
moduleDefinitions.push(moduleDefinition);
if (moduleDefinition.imports.length) {
moduleDefinition.imports.forEach((importStatement) =>
addModuleMetadata(metadata, "sap.ui.define", importStatement));
} else {
// Empty sap.ui.define (no imports, no body)
addModuleMetadata(metadata, "sap.ui.define");
}
pruneNode(node); // Mark the define call for removal
} catch (err) {
if (err instanceof UnsupportedModuleError) {
Expand All @@ -127,6 +134,8 @@ function transform(
if (requireExpression.async) {
const res = transformAsyncRequireCall(node, requireExpression, nodeFactory);
requireImports.push(...res.imports);
res.imports.forEach((importStatement) =>
addModuleMetadata(metadata, "sap.ui.require", importStatement));
if (res.callback) {
replaceNode(node, res.callback);
if (res.errback) {
Expand All @@ -149,6 +158,7 @@ function transform(
} else {
const res = transformSyncRequireCall(node, requireExpression, nodeFactory);
requireImports.push(res.import);
addModuleMetadata(metadata, "sap.ui.require", res.import);
replaceNode(node, res.requireStatement);
}
} catch (err) {
Expand Down Expand Up @@ -273,6 +283,17 @@ function transform(
}
});

function addModuleMetadata(metadata: LintMetadata, importType: string, importStatement?: ts.ImportDeclaration) {
if (!metadata.transformedImports) {
metadata.transformedImports = new Map<string, Set<string>>();
}
const curResource = metadata.transformedImports.get(importType) ?? new Set<string>();
if (importStatement && ts.isStringLiteral(importStatement.moduleSpecifier)) {
curResource.add(importStatement.moduleSpecifier.text);
}
metadata.transformedImports.set(importType, curResource);
}

function getCommentsFromNode(node: ts.Node, sourceFile?: ts.SourceFile): NodeComments {
const sourceText = sourceFile?.getFullText() ?? fullSourceText;
const leadingComments = ts.getLeadingCommentRanges(sourceText, node.getFullStart()) ?? [];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>QUnit test suite for Todo App</title>
<script
src="../resources/sap/ui/test/starter/createSuite.js"
data-sap-ui-testsuite="test-resources/sap/f/testsuite.qunit"
data-sap-ui-resource-roots='{
"test-resources.sap.f": "./"
}'
></script>
</head>
<body>
</body>
</html>
21 changes: 21 additions & 0 deletions test/fixtures/linter/projects/sap.f/test/sap/f/testsuite.qunit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
sap.ui.define(function () {
"use strict";
return {
name: "QUnit test suite for sap.f",
defaults: {
page: "ui5://test-resources/sap/f/Test.qunit.html?testsuite={suite}&test={name}",
qunit: {
version: 2
},
sinon: {
version: 4
},
ui5: {
language: "EN",
theme: "sap_horizon"
}
},
tests: {
}
};
});
13 changes: 11 additions & 2 deletions test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,17 @@ Generated by [AVA](https://avajs.dev).
errorCount: 0,
fatalErrorCount: 0,
filePath: 'Configuration.beforeBootstrap.qunit.js',
messages: [],
warningCount: 0,
messages: [
{
column: 1,
line: 3,
message: 'To save boilerplate code and ensure compliance with UI5 2.x best practices, please migrate to the Test Starter concept',
messageDetails: 'Test Starter (https://ui5.sap.com/#/topic/032be2cb2e1d4115af20862673bedcdb)',
ruleId: 'prefer-test-starter',
severity: 1,
},
],
warningCount: 1,
},
]

Expand Down
Binary file modified test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap
Binary file not shown.
Loading

0 comments on commit 8d7442f

Please sign in to comment.