test: replicate incorrect bundling and missing assertions part of #1323
Description
- Fixes missing assertion on spec to be
trueinbundle.test.ts. Existing tests also failing. - Replicates the problem described in #1323
Related issue(s)
Replicates #1323
I've pulled and synced the changes from master branch into here, and the good news is after #1415 (@aeworxet), the original issue mentioned in #1323 seems solved.
However, the corrected tests are now showing 1 new issue coming probably from the bundler.
The bundler, since 0.5.0, makes it clear that objects must be dereferenced to the maximum extent possible. At least that's how I'm interpreting this.
However, when running the tests, for ASyncAPI v3.0.0 docs, it's leaving the following behind:
components:
messages:
TestMessage:
payload:
type: string
x-origin: '#/components/messages/TestMessage'
This happens for both x-origin use and without. In case of x-origin, it does get referenced through x-origin itself, but I believe this shouldn't matter. In fact, with x-origin: false, then there's no references at all, yet still present. The content gets dereferenced successfully, just not cleaned up.
Right now this is the only thing keeping it failing.
@peter-rr @Amzani @Souvikns @Shurtu-gal any thoughts how you'd like to proceed?
Options I can think of:
- Raise it as a separate bug, and for now I remove the 'offending' part from the test comparison, and this gets solved.
- It's not a bug (please verify my interpretation), so will simply fix the test and this gets solved.
- Get it resolved in bundler and keep this PR waiting until that gets fixed.
Thanks!
@francocm
I am afraid this is not an issue of Bundler, because Bundler doesn't have such complex logic that could lead to such errors. JSON schema is simply transferred to @apidevtools/json-schema-ref-parser with several conditions set, so this issue is most probably the doing of that package.
I have traced the issue up to the crawl() function in @apidevtools/json-schema-ref-parser and opened an issue in its repository.
@aeworxet
Thanks for the analysis.
Worth adding that although this component is ignored, when x-origin: true is used, it's still being updated, as in that scenario, the x-origin: '#/components/messages/TestMessage' gets added, pointing to itself.
I have updated my PR to consider this outcome as expected in the tests, since the scope of this PR is on the issue raised in #1323 and:
- assertions that replicated the original problem
- assertions that were not actually being checked on existing tests
- will now act as better regression test library for future changes
Probably anything else needs to be done in a separate place, and could be tracked in a new issue + linked to the https://github.com/APIDevTools/json-schema-ref-parser/issues/349 you raised.
@peter-rr I've updated the PR with feedback, tests should be passing, and it's no longer a draft. Kindly let me know your thoughts at this stage.
Thanks 🙂
/update
Probably anything else needs to be done in a separate place, and could be tracked in a new issue + linked to the APIDevTools/json-schema-ref-parser#349 you raised.
Yeah, I agree with this 👍 Once the new tests are passing I think we could proceed to close the bug related https://github.com/asyncapi/cli/issues/1323
/update
Hi @peter-rr,
Thanks for following up. I've run npm run lint:fix and pushed it back.
Running npm run test seems to be running fine here?
129 passing (45s)
Full log:
➜ asyncapi-cli git:(fix/1323/bundleNotBundlingExternalFiles) ✗ npm run test
> @asyncapi/[email protected] pretest
> npm run build
> @asyncapi/[email protected] build
> rimraf lib && node scripts/fetch-asyncapi-example.js && tsc && oclif manifest && echo "Build Completed"
Fetched ZIP file
Unzipped all examples from zip
› Warning: oclif update available from 4.5.0 to 4.8.8.
wrote manifest to /Users/<username_redacted>/workspace/asyncapi-cli/oclif.manifest.json
Build Completed
> @asyncapi/[email protected] test
> npm run test:unit
> @asyncapi/[email protected] test:unit
> cross-env NODE_ENV=development TEST=1 CUSTOM_CONTEXT_FILENAME="test.asyncapi-cli" CUSTOM_CONTEXT_FILE_LOCATION="" nyc --extension .ts mocha --require ts-node/register --require test/helpers/init.js --reporter spec --timeout 100000 "test/**/*.test.ts"
bundle
✔ should throw error message if the file path is wrong
bundle spec v3
config:analytics
with disable flag
with enable flag
with no flags
with status flag
config
config:versions
config:context, positive scenario
config:context:current
config:context:list
config:context:add
config:context:add
config:context:edit
config:context:use
config:context:remove
config:context:init
config:context:init
config:context:init
config:context:init
config:context, negative scenario
config:context:add
config:context:add
config:context:add
config:context:add
config:context, negative scenario
config:context:list
convert
with file paths
with no arguments
with no context file
with target-version flag
with output flag
diff
should handle AsyncAPI v3 document correctly
with file paths, and there are no difference between the files
yaml output: with file paths, and there are no difference between the files
with file paths, and getting all changes
with file paths, and getting breaking changes
with file paths, and getting non-breaking changes
with file paths, and getting unclassified changes
with file paths, and getting all changes, passing flag
YAML output, getting all changes
should show error on breaking changes
Markdown output with subtype as json, getting all changes
Markdown output with subtype as yaml, getting all changes
Other output with markdownSubtype flag provided, check for warning
with logging diagnostics
with custom standard file
should throw error for invalid override path
should throw error for invalid override json
template
should handle AsyncAPI v3 document correctly
git clash
Generation failed
✔ should throw error if output folder is in a git repository (205ms)
custom params
disable-hooks
debug
no-overwrite
should install template
Generator Error: Installation failed
map-base-url
models
should handle AsyncAPI v3 document correctly
for TypeScript
for JavaScript
for Python
for Rust
for C#
for C++
for Java
for Go
for Kotlin
for Dart
for PHP
with logging diagnostics
new
create new file
when asyncapi file already exists
new glee
creation of new project is successful
when new project name already exists
optimize
no optimization needed
with no arguments
with no context file
no-tty flag
interactive terminal
error if the asyncapi file is invalid
validate
with file paths
with context names
with no arguments
with no context file
with --log-diagnostics flag
with --diagnostics-format flag
with --fail-severity flag
129 passing (45s)
-----------------------------|---------|----------|---------|---------|----------------------------------------------------------------------------------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
-----------------------------|---------|----------|---------|---------|----------------------------------------------------------------------------------------------
All files | 59.91 | 40.2 | 57.22 | 60.09 |
lib | 79.7 | 51.54 | 87.5 | 79.5 |
base.js | 83.9 | 56.75 | 100 | 83.9 | 63-64,94-95,132,140-153
flags.js | 100 | 100 | 100 | 100 |
globals.js | 38.7 | 0 | 0 | 38.7 | 13-15,20-45
parser.js | 89.74 | 60.97 | 100 | 89.47 | 96-98,100-102,121,124
lib/commands | 90.9 | 73.71 | 100 | 90.82 |
bundle.js | 86.48 | 55.55 | 100 | 86.48 | 26-30,57-58
convert.js | 94.87 | 77.77 | 100 | 94.87 | 54,60
diff.js | 88.49 | 78.33 | 100 | 88.49 | 49,71,90,107,117,138,182-184,186,194,229,256
optimize.js | 92.37 | 73.4 | 100 | 92.17 | 87,94-104,113
validate.js | 95.65 | 75 | 100 | 95.65 | 18
lib/commands/config | 86.88 | 76.47 | 100 | 86.44 |
analytics.js | 80.55 | 69.23 | 100 | 80.55 | 35-48
versions.js | 96 | 100 | 100 | 95.65 | 26
lib/commands/config/context | 75.64 | 17.77 | 100 | 75.64 |
add.js | 84 | 50 | 100 | 84 | 19-20,25-26
current.js | 58.33 | 7.69 | 100 | 58.33 | 16-29
edit.js | 70.83 | 0 | 100 | 70.83 | 19-27
init.js | 100 | 100 | 100 | 100 |
list.js | 86.36 | 50 | 100 | 86.36 | 14-15,28
remove.js | 69.56 | 0 | 100 | 69.56 | 18-26
use.js | 69.56 | 0 | 100 | 69.56 | 18-26
lib/commands/generate | 67.05 | 57.79 | 58.13 | 67.15 |
fromTemplate.js | 59.67 | 44.73 | 60 | 59.56 | 39,49-68,119-120,130-139,145,148,155-163,169,187-189,195-196,207,222,225,241,247,258,277-339
models.js | 75.79 | 70.51 | 53.84 | 76.12 | 37-41,49-51,59,68,170,173,176,214,235-273
lib/commands/new | 49.47 | 20.77 | 45.83 | 49.47 |
file.js | 55.17 | 24.48 | 66.66 | 55.17 | 33,39-113,122,130-131,143
glee.js | 42.57 | 14.28 | 33.33 | 42.42 | 39-117,130,133-134,144-152,163
index.js | 100 | 100 | 100 | 100 |
lib/errors | 65.81 | 31.25 | 65.21 | 66.07 |
context-error.js | 83.78 | 100 | 69.23 | 87.5 | 60-61,67-68
diff-error.js | 100 | 100 | 100 | 100 |
generator-error.js | 100 | 100 | 100 | 100 |
specification-file.js | 54.83 | 58.33 | 25 | 54.83 | 7-18,25-26,41-42,45-46,49-50
validation-error.js | 31.03 | 15 | 50 | 31.03 | 8,14,19-48
lib/models | 69.66 | 60.9 | 70.51 | 69.6 |
Context.js | 80.28 | 61.66 | 100 | 80.14 | 74-75,88,107,133,136,150,164,167,182,207,227,245-267,291-294
SpecificationFile.js | 82.64 | 72.13 | 84.37 | 82.35 | 43,56,75,86-90,99-106,117,120,123,140,143,159,188-193,220
Studio.js | 25.71 | 0 | 0 | 26.08 | 20-101,106-123
lib/utils | 12.67 | 0 | 0 | 13.04 |
generator.js | 12.67 | 0 | 0 | 13.04 | 11-13,19-149
src | 14.92 | 0 | 0 | 15.15 |
base.ts | 14.92 | 0 | 0 | 15.15 | 20-132
src/commands | 21.42 | 4.25 | 22.22 | 22.01 |
optimize.ts | 21.42 | 4.25 | 22.22 | 22.01 | 27,49-210
src/errors | 20.87 | 0 | 0 | 20.87 |
context-error.ts | 40 | 100 | 0 | 40 | 11,13,15,17,19-24,30-31,37-38,44-45,51-52,58-59,65-66,72-73
specification-file.ts | 13.79 | 0 | 0 | 13.79 | 4-5,11-15,22-23,30-54
validation-error.ts | 3.7 | 0 | 0 | 3.7 | 11-57
src/models | 23.82 | 7.43 | 10.52 | 24.22 |
Context.ts | 27.77 | 15 | 13.33 | 27.2 | 85-121,125-139,143-153,157-170,174-182,189-200,208-215,223-260,280-309,335-340,351-358,368
SpecificationFile.ts | 19.26 | 0 | 8.69 | 20.58 | 27-97,113,125-155,160-171,176-180,185-223
-----------------------------|---------|----------|---------|---------|----------------------------------------------------------------------------------------------
> @asyncapi/[email protected] posttest
> rimraf ./test.asyncapi-cli
➜ asyncapi-cli git:(fix/1323/bundleNotBundlingExternalFiles) ✗
Hello, @francocm! 👋🏼
I'm 🧞🧞🧞 Genie 🧞🧞🧞 from the magic lamp. Looks like somebody needs a hand!
At the moment the following comments are supported in pull requests:
- `/please-take-a-look` or `/ptal` - This comment will add a comment to the PR asking for attention from the reviewrs who have not reviewed the PR yet.
- `/ready-to-merge` or `/rtm` - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
- `/do-not-merge` or `/dnm` - This comment will block automerging even if all conditions are met and ready-to-merge label is added
- `/autoupdate` or `/au` - This comment will add `autoupdate` label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR. (Currently only works for upstream branches.)
- `/update` or `/u` - This comment will update the PR with the latest changes from the target branch. Unless there is a merge conflict or it is a draft PR. NOTE: this only updates the PR once, so if you need to update again, you need to call the command again.
Just realised, if it helps I'm using Node 20:
➜ asyncapi-cli git:(fix/1323/bundleNotBundlingExternalFiles) ✗ node --version
v20.12.2
➜ asyncapi-cli git:(fix/1323/bundleNotBundlingExternalFiles) ✗ npm --version
10.5.0
➜ asyncapi-cli git:(fix/1323/bundleNotBundlingExternalFiles) ✗
Just realised, if it helps I'm using Node 20
Thanks @francocm for the tip 👍 I've tried with v20 but still getting the errors. Also now I'm getting many errors during the building process after updating CLI to the latest version 1.14.0. Even working on master branch. Let me investigate why I'm getting those results 🤔 and I'll get back to you.
@peter-rr
Can you please try cloning, npm-installing, and running your tests from zero? Sometimes there are manual changes you completely forgot you made, and they are not displayed with git status because files are in .gitignore.
Knowing the logic of Bundler, it explicitly compares the integer part of the asyncapi property of the second AsyncAPI Document with the integer part of the asyncapi property of the first AsyncAPI Document, and it is very hard to compare two integers incorrectly if no changes are introduced somewhere else in the code.
Thanks @aeworxet 😄 I just needed npm-installing to include the latest version of @oclif/core dependency. Everything working fine now.
@francocm LGTM 👍 All tests passing after the latest changes applied.
Now this PR needs to be reviewed by the codeowners before being merged.
/ptal
@Amzani @Souvikns @Shurtu-gal Please take a look at this PR. Thanks! :wave:
/update
/update
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code
This pull request has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.
There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.
Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.
Thank you for your patience :heart:
still relevant
@francocm are you still working on this?
Closing this ref: https://github.com/asyncapi/cli/issues/1323#issuecomment-2579882693
UPDATE https://github.com/APIDevTools/json-schema-ref-parser/issues/349#issuecomment-2628833419