Fix: diff command (@W-19076728@)

[joeluong-sfcc]

This PR modifies the behavior of the diff command, where if the directory name contains the version number, it'll remove the minor and the patch version. For example:

• Before: shopper-stores-oas-1.0.16
• After: shopper-stores-oas-1

To disable this behavior, a user can pass in the --disable-normalize-directory-names flag.

This is because when we run the updateApis command, we append the version number. The current implementation of the diff command on directory mode looks for matches in directory names, but since directories have the version number, there will never be a match unless the API version does not change. Example diff file:

======assignments-oas-1.0.34 API is deleted======
======campaigns-oas-1.0.36 API is deleted======
======catalogs-oas-1.0.39 API is deleted======
...
======assignments-oas-1.0.35 API is added======
======campaigns-oas-1.0.37 API is added======
======catalogs-oas-1.0.41 API is added======

By normalizing the directory names to only include the major version in the directory name, we can start to compare the old API spec with the new API spec. We keep the major version for APIs with multiple versions, like shopper baskets. Example diff file with with updated normalization behavior:

=== Changes in shopper-baskets-oas-1 ===
--- Changes in shopper-baskets-oas-v1-public.yaml ---
...

=== Changes in shopper-baskets-oas-2 ===
--- Changes in shopper-baskets-oas-v2-public.yaml ---
...

This PR also removes some unnecessary test files that weren't being used.

To test this PR:

1. in RAML toolkit, git checkout ju/fix-diff-command
2. npm run build
3. Checkout this isomorphic SDK PR: Remove internal OAS file and Fix generation script (@W-19076728@) commerce-sdk-isomorphic#208 - git checkout ju/fix-diff-command
4. Set up ANYPOINT_USERNAME and ANYPOINT_PASSWORD env variables via shared password vault
5. In isomorphic SDK: yarn ci && yarn updateApis
6. verify in temp/diffApis.txt that diff shows up

[joeluong-sfcc]

Was seeing an error in isomorphic SDK where we tried to read version property from undefined

[vcua-mobify]

Aside from a question around whether this should be enabled by default, the changes seem good to me.

Followed the test steps and the resulting diff file was produced

[vcua-mobify]

Just wondering; wouldn't the most common use case have us set this to true so that we can compare the difference between two minor versions of the same API?

Otherwise, the diff will say the old version is removed and the new version is added and leave it at that, no? I don't think this old-version-removed/new-version-added also does a file by file compare?

[alexvuong]

agreed, this should be the normal behavior, we should remove the patch verion alltoger since it is not possible the api will do release the same patch number twice. the minimal is the minor version (default): v1.1 vs v1.2 and the the other is v1 or v2. with this, any 1.1.1 and 1.1.4 can still produce meaningful output for us

[joeluong-sfcc]

Updated the PR so that normalizing the directory name is the default behavior, you can pass in the --disable-normalize-directory-names flag to override this behavior.

For now, I'll keep the patch number appended to the directory name as part of the releases we're going to start to tie API versions to each SDK version, see this draft PR for an example: https://github.com/SalesforceCommerceCloud/commerce-sdk/pull/427/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR5

[vcua-mobify]

Thanks for making this change, @joeluong-sfcc !

[alexvuong]

This set up is used to silent all console logs in tests. Are you sure you want to remove it?

[joeluong-sfcc]

Ah, good point, I didn't think about silencing the console logs. I went back to my original approach where I export the consoleStub

[alexvuong]

Nit: For consistency, in previous tests, we are assertting the exStub arg for the command output. Like this

expect(execStub.called).to.be.true;
    expect(execStub.args[1][0]).to.equal(
      'oasdiff changelog "base/api-v1/spec.yaml" "new/api-v1/spec.yaml"'
    );

Any reason we don't want to do it here?

[joeluong-sfcc]

Yeah, it's because if I target the expected string in the consoleStub directly it looks like this:

    expect(consoleStub.args[2][0]).to.equal("Processing directory pair: api-1");
    expect(consoleStub.args[6][0]).to.equal("Processing directory pair: api-2");

If we add a console.log somewhere else in the code path, this test will break, I used .find() to make it more resilient.

I've updated the test however flatten the args array so its a bit cleaner