move Freestyle OPA templates to ui5-test-writer
#2928
Conversation
🦋 Changeset detectedLatest commit: b3c74b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…n-ux-tools into freestyle-test-templates
…n-ux-tools into freestyle-test-templates
| }) | ||
| }; | ||
| if (addTests) { | ||
| const ui5MockYamlScript = addMock ? '--config ./ui5-mock.yaml ' : ''; |
There was a problem hiding this comment.
Can you refactor this conditional block into a single function, in another file, please to encapsulate all the updates in a single place. We should try and keep this function (generate) at the same level of abstraction for clarity and understandability. Perhaps this will reduce the code smell (complexity > 15) https://sonarcloud.io/project/issues?id=SAP_open-ux-tools&pullRequest=2928&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true
There was a problem hiding this comment.
Thanks, can we include all the test related code in a single function and move to another file perhaps to try and keep this file under a manageable size and at the same level of abstraction.
packages/fiori-elements-writer/test/__snapshots__/lrop.test.ts.snap
Outdated
Show resolved
Hide resolved
| window.suite = function() { | ||
| 'use strict'; | ||
| 'use strict'; |
There was a problem hiding this comment.
Formatting looks wrong here? These changes will impact on existing FE snapshots. I would assume the previous formatting was correct, so probably best to leave it as is.
|
@IainSAP @devinea Previously I had combined |
| } | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this file can be removed as it was related to handlebars?
| ...opaConfig, | ||
| viewNamePage: `${viewName}Page`, | ||
| appIdWithSlash, | ||
| ui5Version: templateUi5Version, |
There was a problem hiding this comment.
| ui5Version: templateUi5Version, | |
| templateUi5Version: templateUi5Version, |
| <link rel="stylesheet" type="text/css" href="../../resources/sap/ui/thirdparty/qunit-2.css"> | ||
| <script src="../../resources/sap/ui/thirdparty/qunit-2.js"></script> | ||
| <script src="../../resources/sap/ui/qunit/qunit-junit.js"></script> | ||
| <% if (ui5Version !== "1.71.0") { %><script src="opaTests.qunit.js"></script><% } -%> |
There was a problem hiding this comment.
| <% if (ui5Version !== "1.71.0") { %><script src="opaTests.qunit.js"></script><% } -%> | |
| <% if (templateUi5Version !== "1.71.0") { %><script src="opaTests.qunit.js"></script><% } -%> |
I think it is better to rename this variable for clarity
| @@ -0,0 +1,17 @@ | |||
| /* global QUnit */ | |||
| <% if (ui5Version === '1.71.0') { -%> | |||
There was a problem hiding this comment.
| <% if (ui5Version === '1.71.0') { -%> | |
| <% if (templateUi5Version === "1.71.0") { -%> |
| // eslint-disable-next-line fiori-custom/sap-no-global-define | ||
|
|
||
| window.suite = function() { | ||
| 'use strict'; |
There was a problem hiding this comment.
Must use double quotes in UI5 template files
…n-ux-tools into freestyle-test-templates
| ui5Version: ffApp.ui5?.version, | ||
| enableTypeScript: ffApp.appOptions?.typescript | ||
| }; | ||
| await generateFreestyleOPAFiles(basePath, config, fs, log); |
There was a problem hiding this comment.
Why not put all the test related code (in the condition) into a single function (in another file), this keeps this file cleaner and will facilitate unit testing of the test related code :
if (addTests) {
generateOPATests()
}
| * @param {Package} packageJson - The package.json object to update. | ||
| * @param {boolean} addMock - Whether to include the UI5 mock YAML configuration. | ||
| */ | ||
| function addTestScripts(packageJson: Package, addMock: boolean): void { |
There was a problem hiding this comment.
Please consider moving this to another function in another file along with all the test related code.
|
#2932
ui5-writer-testsad logic.addUnitsflag to freestyle Fiori App to enable tests.