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

Feat add specific unit and opa5 test templates #78

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

heimwege
Copy link

@heimwege heimwege commented Jul 16, 2024

In case the project is a TypeScript project the generator should generate .ts files for opa5 and unit tests

Out of scope for this PR: In case the project is a Fiori Elements (fpmpage) project the generator should make use of sap.fe.test in the generated opa5 tests

Depends on SAP/open-ux-tools#2164

@nicoschoenteich
Copy link
Collaborator

nicoschoenteich commented Jul 16, 2024

Looks good!

Does this mean we should merge this and SAP/open-ux-tools#2164 at the same time? Or merge the open-ux-tools first and make sure this generator uses the latest version after its release?

@heimwege
Copy link
Author

Looks good!

Does this mean we should merge this and SAP/open-ux-tools#2164 at the same time? Or merge the open-ux-tools first and make sure this generator uses the latest version after its release?

I would wait till the preview-middleware fix is in. Won't take that long 🤞🏻

heimwege added 2 commits July 18, 2024 08:11
… feat-add-specific-unit-and-opa5-test-templates

# Conflicts:
#	generators/dependencies.js
@nicoschoenteich
Copy link
Collaborator

Thanks for the newest commits. Just waiting for SAP/open-ux-tools#2164 and will merge after that.

@heimwege
Copy link
Author

heimwege commented Jul 22, 2024

@nicoschoenteich luckily we waited. I'll adjust the templates according to what @petermuessig just implemented 🐱

UPDATE: now that I had a look at it it's not relevant for the templates because everything that changed is served via preview-middleware 🥳

@heimwege
Copy link
Author

@nicoschoenteich I just merged SAP/open-ux-tools#2164 so 0.16.23 should be the right version for the preview middleware but I don't find where it is maintained in the generator 🕵🏻 Or is it only an indirect dependency of @sap/ux-ui5-tooling? For testing purposes I added it to the package.json of the generated ui app and everything seems to work with the ts template. But please double check as well 😸

Copy link
Member

@petermuessig petermuessig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few learnings from today

generators/opa5/templates/Journey.ts Outdated Show resolved Hide resolved
generators/opa5/templates/Journey.ts Outdated Show resolved Hide resolved
generators/opa5/templates/Journey.ts Show resolved Hide resolved
@heimwege heimwege requested a review from petermuessig July 24, 2024 17:09
@heimwege heimwege marked this pull request as ready for review July 25, 2024 12:36
… feat-add-specific-unit-and-opa5-test-templates

# Conflicts:
#	generators/dependencies.js
Copy link
Member

@petermuessig petermuessig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - just one small comment

generators/dependencies.js Outdated Show resolved Hide resolved
generators/opa5/index.js Outdated Show resolved Hide resolved
generators/qunit/index.js Outdated Show resolved Hide resolved
@heimwege
Copy link
Author

@nicoschoenteich not sure what this error is all about 🤷🏻

npm error Lifecycle script `ts-typecheck` failed with error:
npm error Error: command failed
npm error   in workspace: myui5app@0.0.1
npm error   at location: /home/runner/work/generator-ui5-project/generator-ui5-project/test-output/com.myorg.myui5project/myui5app
npm error Lifecycle script `build` failed with error:
npm error Error: command failed
npm error   in workspace: myui5app@0.0.1
npm error   at location: /home/runner/work/generator-ui5-project/generator-ui5-project/test-output/com.myorg.myui5project/myui5app
make: *** [Makefile_20241028135626.mta:28: pre_build] Error 1
Error: could not build the MTA project: could not execute the "make -f Makefile_20241028135626.mta p=cf mtar=com.myorg.myui5project.mtar strict=true mode=" command: exit status 2

      at checkExecSyncError (node:child_process:890:11)
      at execSync (node:child_process:962:15)
      at Context.<anonymous> (file:///home/runner/work/generator-ui5-project/generator-ui5-project/test/all-projects.js:162:11)
      at process.processImmediate (node:internal/timers:476:21)

@nicoschoenteich
Copy link
Collaborator

I think a new version of the template now runs tsc as part of a ts-typecheck script, so we would need tsc as a dev dependency if this.options.config.enableTypescript. I think this shouldn't be fixed here, but rather on the template side in @sap-ux/ui5-application-writer.

@heimwege
Copy link
Author

I think a new version of the template now runs tsc as part of a ts-typecheck script, so we would need tsc as a dev dependency if this.options.config.enableTypescript. I think this shouldn't be fixed here, but rather on the template side in @sap-ux/ui5-application-writer.

The tsc command comes with the typescript package. And this is part of the core templates for typescript/package.json. So I would have expected it is already in 🤔

@nicoschoenteich
Copy link
Collaborator

You are right. This particular test also passes locally (after uninstalling typescript globally). Not sure what is going on here - we might just merge anyway. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants