test: replicate incorrect bundling and missing assertions part of #1323 #1389

francocm · 2024-04-24T11:56:11Z

Description

Fixes missing assertion on spec to be true in bundle.test.ts. Existing tests also failing.
Replicates the problem described in [BUG] The bundle command does not bundle referenced files #1323

Related issue(s)

Replicates #1323

github-actions

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

peter-rr

Thanks for raising this draft! I left some comments :)

test/integration/bundle/bundle.test.ts

…#1323

test/integration/bundle/bundle.test.ts

peter-rr

Left some comments 😄

test/integration/bundle/bundle.test.ts

francocm · 2024-05-09T15:59:20Z

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!

aeworxet · 2024-05-13T00:53:29Z

@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.

…ope of this PR will ignore the extra component block

…omments in asyncapi#1389.

francocm · 2024-05-13T12:33:06Z

@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 APIDevTools/json-schema-ref-parser#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 🙂

peter-rr · 2024-05-14T17:08:22Z

/update

peter-rr

Hey @francocm, great work! 😄
The changes you have applied look good to me. However, all the tests related to bundle command are not passing since I'm getting this error when executing the command for any test case:

Error: Unable to bundle specification file of different major versions

I've updated your branch with the latest changes from master but can't make the tests pass. Could you please try to run the tests again after updating your branch to check the result?

Also we're getting this error on the CI workflow related to linter. You can fix it easily by running npm run lint:fix

peter-rr · 2024-05-14T17:52:03Z

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 #1323

peter-rr · 2024-05-15T09:00:38Z

/update

francocm · 2024-05-15T09:56:39Z

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) ✗

asyncapi-bot · 2024-05-15T09:56:53Z

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.

francocm · 2024-05-15T09:57:40Z

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) ✗

peter-rr · 2024-05-15T10:26:43Z

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.

aeworxet · 2024-05-15T11:39:31Z

@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.

peter-rr · 2024-05-16T16:00:24Z

Thanks @aeworxet 😄 I just needed npm-installing to include the latest version of @oclif/core dependency. Everything working fine now.

peter-rr · 2024-05-16T16:10:46Z

@francocm
LGTM 👍 All tests passing after the latest changes applied.

Now this PR needs to be reviewed by the codeowners before being merged.

peter-rr · 2024-05-16T16:11:19Z

/ptal

asyncapi-bot · 2024-05-16T16:11:32Z

@Amzani @Souvikns @Shurtu-gal Please take a look at this PR. Thanks! 👋

peter-rr · 2024-05-16T16:11:41Z

/update

Shurtu-gal · 2024-05-17T14:09:00Z

/update

sonarcloud · 2024-05-17T14:09:46Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Shurtu-gal · 2024-05-17T14:15:43Z

test/integration/bundle/final-asyncapi-with-xorigin.yaml

@francocm could you add the spec files in test/fixtures as can be seen for others.

test: replicate incorrect bundling and missing assertions part of asy…

b432838

…ncapi#1323

github-actions bot reviewed Apr 24, 2024

View reviewed changes

francocm mentioned this pull request Apr 24, 2024

[BUG] The bundle command does not bundle referenced files #1323

Open

2 tasks

peter-rr suggested changes Apr 25, 2024

View reviewed changes

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

francocm added 2 commits April 26, 2024 19:54

Updated assertion styles as per PR feedback as part of asyncapi#1323

03ce3a6

Fixed incorrect test and incorrect expected files as part of asyncapi…

e719f24

…#1323

peter-rr reviewed Apr 30, 2024

View reviewed changes

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

peter-rr reviewed Apr 30, 2024

View reviewed changes

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

peter-rr reviewed Apr 30, 2024

View reviewed changes

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

peter-rr suggested changes Apr 30, 2024

View reviewed changes

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

test/integration/bundle/bundle.test.ts Outdated Show resolved Hide resolved

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

c8564ad

aeworxet mentioned this pull request May 8, 2024

fix: update @asyncapi/bundler to 0.5.0 version #1415

Merged

francocm added 2 commits May 9, 2024 02:53

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

d1c5c19

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

1becec0

francocm added 2 commits May 13, 2024 11:59

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

d4296c5

As per issue raised in APIDevTools/json-schema-ref-parser#349 this sc…

104affb

…ope of this PR will ignore the extra component block

francocm marked this pull request as ready for review May 13, 2024 11:09

francocm requested review from Souvikns, Amzani, Shurtu-gal and asyncapi-bot-eve as code owners May 13, 2024 11:09

Moved back to validateGeneratedSpec with explicit expects as per PR c…

3bcdee9

…omments in asyncapi#1389.

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

8efa7cf

peter-rr suggested changes May 14, 2024

View reviewed changes

asyncapi-bot and others added 2 commits May 15, 2024 11:00

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

c06a4d4

lint fix

c761c8c

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

1fa8a93

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

523b5f2

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

bb0c1fd

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

041f834

Merge branch 'master' into fix/1323/bundleNotBundlingExternalFiles

3fcec67

Shurtu-gal requested changes May 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: replicate incorrect bundling and missing assertions part of #1323 #1389

test: replicate incorrect bundling and missing assertions part of #1323 #1389

francocm commented Apr 24, 2024

github-actions bot left a comment

peter-rr left a comment

peter-rr left a comment

francocm commented May 9, 2024

aeworxet commented May 13, 2024

francocm commented May 13, 2024

peter-rr commented May 14, 2024

peter-rr left a comment

peter-rr commented May 14, 2024 •

edited

Loading

peter-rr commented May 15, 2024

francocm commented May 15, 2024

asyncapi-bot commented May 15, 2024

francocm commented May 15, 2024

peter-rr commented May 15, 2024

aeworxet commented May 15, 2024

peter-rr commented May 16, 2024

peter-rr commented May 16, 2024

peter-rr commented May 16, 2024

asyncapi-bot commented May 16, 2024

peter-rr commented May 16, 2024

Shurtu-gal commented May 17, 2024

sonarcloud bot commented May 17, 2024

Shurtu-gal May 17, 2024

test: replicate incorrect bundling and missing assertions part of #1323 #1389

Are you sure you want to change the base?

test: replicate incorrect bundling and missing assertions part of #1323 #1389

Conversation

francocm commented Apr 24, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

peter-rr left a comment

Choose a reason for hiding this comment

peter-rr left a comment

Choose a reason for hiding this comment

francocm commented May 9, 2024

aeworxet commented May 13, 2024

francocm commented May 13, 2024

peter-rr commented May 14, 2024

peter-rr left a comment

Choose a reason for hiding this comment

peter-rr commented May 14, 2024 • edited Loading

peter-rr commented May 15, 2024

francocm commented May 15, 2024

asyncapi-bot commented May 15, 2024

francocm commented May 15, 2024

peter-rr commented May 15, 2024

aeworxet commented May 15, 2024

peter-rr commented May 16, 2024

peter-rr commented May 16, 2024

peter-rr commented May 16, 2024

asyncapi-bot commented May 16, 2024

peter-rr commented May 16, 2024

Shurtu-gal commented May 17, 2024

sonarcloud bot commented May 17, 2024

Quality Gate passed

Shurtu-gal May 17, 2024

Choose a reason for hiding this comment

peter-rr commented May 14, 2024 •

edited

Loading