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

[Draft] Update on script npm run generator:test:cli #1369

Open
Adi-204 opened this issue Feb 20, 2025 · 6 comments · May be fixed by #1383
Open

[Draft] Update on script npm run generator:test:cli #1369

Adi-204 opened this issue Feb 20, 2025 · 6 comments · May be fixed by #1383

Comments

@Adi-204
Copy link
Contributor

Adi-204 commented Feb 20, 2025

While running npm run generator:test:cli the script does not throw any errors but results in the following output:

Image

The last package update introduced the deprecation of ag. If no task are executed by the script should we proceed with removing this script from package.json and the documentation, or are there any team discussions/plans regarding this?
Please provide feedback and explanation.

@Adi-204
Copy link
Contributor Author

Adi-204 commented Feb 20, 2025

@derberg sir can you please provide feedback thanks : )

Copy link
Member

derberg commented Feb 20, 2025

hey, the fact that we deprecated cli do not mean it is removed - cli test should be invoked - it is simple cli run - but should run afaik. Did you try to investigate why it doesn't run? maybe we have an error in turborepo config?

@Adi-204
Copy link
Contributor Author

Adi-204 commented Feb 20, 2025

@derberg sir on investigate I found a PR https://github.com/asyncapi/generator/pull/1315/files in which the test:cli was removed
from apps/generator/package.json link - https://github.com/asyncapi/generator/pull/1315/files#diff-d6d2118910ac9ca291d509fa9f637faf7a33d983b0dc1707663f48efe54f5974

Copy link
Member

derberg commented Feb 20, 2025

ah, ok, as it was using the deprecated CLI
we probably need to figure how to do this using AsyncAPI CLI, maybe with npx 🤔

@Adi-204
Copy link
Contributor Author

Adi-204 commented Feb 21, 2025

@derberg Sir, could you please provide more details about the approach so that I can add value in resolving the issue?
From my understanding, the cli.js file contains code for the deprecated ag CLI, and we have now transitioned to the AsyncAPI CLI. This implies that we are no longer directly testing the cli.js file. Instead, are we planning to run tests directly on the AsyncAPI CLI, as shown below?
"test:cli": "npx @asyncapi/cli ..."
However, this approach does not actually test the code of cli.js file.
Could you please explain thanks : )

@derberg
Copy link
Member

derberg commented Mar 5, 2025

However, this approach does not actually test the code of cli.js file.

very good point. Just that the cli test was not there really to test the cli itself, but rather test generator in most extreme integration test possible. Now yes, problem is that on a CI level, when we run cli test, it is not really testing the current generator, but the last one available in AsyncAPI CLI

and because of above, I agree with you, please complete the removal of CLI test command - it really should be removed

@derberg derberg moved this to Todo in Maintainers work Mar 5, 2025
@Adi-204 Adi-204 linked a pull request Mar 5, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

2 participants