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

fix: @commitlint/parse package logic appears incorrect #3784

Closed
1 of 4 tasks
silversonicaxel opened this issue Nov 20, 2023 · 2 comments
Closed
1 of 4 tasks

fix: @commitlint/parse package logic appears incorrect #3784

silversonicaxel opened this issue Nov 20, 2023 · 2 comments
Labels

Comments

@silversonicaxel
Copy link
Contributor

silversonicaxel commented Nov 20, 2023

Expected Behavior

These two following tests

  • 'scope-enum with multiple scopes should error on message with forbidden enum'
  • 'scope-enum with multiple scopes should error on message with superfluous scope'

coming from https://github.com/conventional-changelog/commitlint/blob/renovate/conventional-changelog-angular-7.x/%40commitlint/rules/src/scope-enum.test.ts are adapting to some wrong behaviour of @commitlint/parse/ package:

It seems that:

  • For 'always', the validation should pass only if the message includes all the given scopes. Otherwise, it should fail. (currently implemented)
  • For 'never', the validation should fail if the message includes any of the given scopes. Otherwise, it should pass.

Here #3725 some reference of the process of finding out the effect of the issue, not the cause.

Fixing this error will require a refactoring of tests here too https://github.com/conventional-changelog/commitlint/blob/renovate/conventional-changelog-angular-7.x/%40commitlint/rules/src/scope-enum.test.ts

Once this error is covered, conventional changelog angular 7.x could be finally released and this could resolve these issues #3698 and release-it/conventional-changelog#70

Current Behavior

This seems a wrong logic.

test('scope-enum with multiple scopes should error on message with forbidden enum', async () => {
  const [actual, message] = scopeEnum(await parsed.multiple, 'never', [
    'bar',
    'qux',
  ]);
  const expected = true;
  expect(actual).toEqual(expected);
});

test('scope-enum with multiple scopes should error on message with superfluous scope', async () => {
  const [actual] = scopeEnum(await parsed.multiple, 'never', ['bar']);
  const expected = true;
  expect(actual).toEqual(expected);
});

Affected packages

  • cli
  • core
  • prompt
  • config-angular

Possible Solution

To investigate

Steps to Reproduce

1. Run `yarn test scope-enum` and check the logic of the tests mentioned above

Context

All starts here release-it/conventional-changelog#70

commitlint --version

18.2.0

git --version

2.39.2 (Apple Git-143)

node --version

v18.18.2

@silversonicaxel
Copy link
Contributor Author

As a follow up @escapedcat and @joberstein

@silversonicaxel
Copy link
Contributor Author

I just saw it here the same bug #3779
So I close this as duplication.

FYI @escapedcat and @joberstein

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

1 participant