feat: integrate asyncapi diff library using the diff command

[aayushmau5]

Description

This PR integrates the asyncapi-dff library using the diff command.

At the moment, the command looks something like this.

asyncapi diff <file-path/context-1> <file-path/context-2>

And it has two flags:

--format(default and only acceptable right now value: json) which determines the format of the output.
--type(all (default) , breaking, non-breaking, unclassified) which determines the type of the output we want.

At the moment, the output is being logged to stdout. The support for outputting data into files is to be added(which is most likely to be done by me, whether in this PR or another one depends on the review :) ).

Related issue(s)

See #58

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

[derberg]

Awesome to see it comming!
Please have a look at some possible improvements to developer experience

[aayushmau5]

Thanks for the reviews. Will make the changes ASAP.

[derberg]

@aayushmau5 hey, tests are failing because of linter.

[Souvikns]

This is something new

[aayushmau5]

Yeah, confused as to why this is happening. Let me check if this has something to do with the context I'm setting in the tests.

[derberg]

@aayushmau5 you need a hand? or two? 😄

[aayushmau5]

Yeah 😅 Not sure why the test fails in windows even though I have not used any context in the tests for diff command 😕

[derberg]

looking at logs Error: EPERM: operation not permitted, open 'C:\Users\runneradmin\.test.asyncapi' looks like tests fail as there is no access to the config file that keeps context, and test that removes it, cannot do it because it has no rights to do it.

super strange that it is happening right now, they had to do some updates to VM or something.

The problem is I have no idea how to solve it, other than adding a NODE_ENV that you can use in tests to override default context location. This way you could provide location that can be easily modified by the workflow.

@Souvikns @boyney123 @magicmatatjahu thoughts?

to summarize, I'm thinking about modifying these 3 lines -> https://github.com/asyncapi/cli/blob/master/src/models/Context.ts#L8-L10

const isTestEnv = !!process.env.TEST;
const CONTEXT_FILENAME = isTestEnv ? '.test.asyncapi' : '.asyncapi';
export const DEFAULT_CONTEXT_FILE_PATH = path.resolve(os.homedir(), CONTEXT_FILENAME);

with something like

const CONTEXT_FILENAME = process.env.CONTEXT_FILENAME || '.asyncapi';
export const DEFAULT_CONTEXT_FILE_PATH = process.env.CONTEXT_FILE_PATH || path.resolve(os.homedir(), CONTEXT_FILENAME);

and then just update test script in package.json to provide custom name and path location

[Souvikns]

const isTestEnv = !!process.env.TEST;
const CONTEXT_FILENAME = isTestEnv ? '.test.asyncapi' : '.asyncapi';
export const DEFAULT_CONTEXT_FILE_PATH = path.resolve(os.homedir(), CONTEXT_FILENAME);

we have this so that while testing we don't overwrite the global config file. I think we should just check if it is test env then we should choose .test.asyncapi or choose something from the env variable.

maybe something like this

const CONTEXT_FILENAME = isTestEnv ? process.env.context_file_path || '.test.asyncapi' : '.asyncapi'

[derberg]

@Souvikns the thing is that we should have no test-related logic in library, this is why I propose that we have all customizable and we just override it in the test script, and we create context config in test dir during testing, and make sure the file is in gitignore

[Souvikns]

@Souvikns the thing is that we should have no test-related logic in library, this is why I propose that we have all customizable and we just override it in the test script, and we create context config in test dir during testing, and make sure the file is in gitignore

Oh now I get it, yeah that would be better

[derberg]

@aayushmau5 your test still runs outside it function, have a look at the context tests and how I changed them

[aayushmau5]

what did we agree on with another flag that is needed, to enable user to pass a path to custom standard?

Oops, I don't think we have discussed this 🤔 . I completely forgot about passing custom standard.

What flag should we go with? Also, I'm assuming this file will be a json file(or we should treat it as such)?

[derberg]

yeah, just enable what API of the diff library allows you to do. So definitely a json file, and I think --overrides is good, just provide good help description

[aayushmau5]

Not sure where to put this function.

[derberg]

imho it is fine if it just stays here, just add js doc

most important is to have proper error handling, awaits should be wrapped in try/catch and we need test, what error goes out when user passed path to not existing file and what error goes out when user passed path to not json file and JSON.parse fails

[aayushmau5]

@derberg It seems adding tests like

it(..., () => {
	it(..., () => {})
})

results in the tests passing no more whether it really fails or not.

But

describe(..., () => {
	it(..., () => {})
})

seems to work fine.

Found this bug when trying to test an error, but the test passed for the wrong error.

[derberg]

@aayushmau5 yes, go ahead with

describe(..., () => {
	it(..., () => {})
})

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[derberg]

@aayushmau5 lgtm 🚀 I guess I can merge, right?

[aayushmau5]

Yes! 🚀

[derberg]

/rtm

[asyncapi-bot]

🎉 This PR is included in version 0.13.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀