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

[ESLint Plugin] Some of the tests behavior changed after TS upgrade #13186

Closed
deyaaeldeen opened this issue Jan 12, 2021 · 5 comments
Closed
Labels
eslint plugin help wanted This issue is tracking work for which community contributions would be welcomed and appreciated

Comments

@deyaaeldeen
Copy link
Member

deyaaeldeen commented Jan 12, 2021

ts-use-promises changed its behavior after upgrading TS to v4.1.2 causing a unit test to fail:

Detailed error

  1 failing
  1) ts-use-promises
       invalid
         import Promise from 'bluebird';
const promise = (): Promise<string> => {
    return new Promise(resolve => resolve("example"));
}
:

      AssertionError [ERR_ASSERTION]: Should have 1 error but had 0: []
      + expected - actual

      -0
      +1

      at testInvalidTemplate (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/rule-tester/rule-tester.js:666:24)
      at Context.<anonymous> (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/eslint/lib/rule-tester/rule-tester.js:893:25)
      at callFn (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runnable.js:374:21)
      at Test.Runnable.run (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runnable.js:361:7)
      at Runner.runTest (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runner.js:619:10)
      at /mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runner.js:745:12
      at next (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runner.js:536:14)
      at /mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runner.js:546:7
      at next (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runner.js:448:14)
      at Immediate._onImmediate (/mnt/vss-agent-2.179.0/_work/1/s/common/temp/node_modules/.pnpm/[email protected]/node_modules/mocha/lib/runner.js:514:5)
      at processImmediate (internal/timers.js:461:21)

Code pointers for anyone wanting to fix this:

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jan 12, 2021
@deyaaeldeen deyaaeldeen self-assigned this Jan 12, 2021
@deyaaeldeen deyaaeldeen added this to the Backlog milestone Jan 12, 2021
@deyaaeldeen deyaaeldeen added eslint plugin and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Jan 13, 2021
@deyaaeldeen deyaaeldeen added the help wanted This issue is tracking work for which community contributions would be welcomed and appreciated label Jul 26, 2021
@ramya-rao-a
Copy link
Contributor

I don't see the above test failing anymore, but I see a different one failing:

  1143 passing (8m)
  1 failing

  1) plugin
       rules
         the number of rules should match the expected value:

      AssertionError: expected 46 to equal 47
      + expected - actual

      -46
      +47

      at Context.<anonymous> (C:\GitRepos\azure-sdk-for-js\common\tools\eslint-plugin-azure-sdk\tests\plugin.ts:157:14)
      at callFn (C:\GitRepos\azure-sdk-for-js\common\temp\node_modules\.pnpm\[email protected]\node_modules\mocha\lib\runnable.js:374:21)

@deyaaeldeen
Copy link
Member Author

@ramya-rao-a the test mentioned in the description is currently commented out:

// this could should be uncommented after this issue has been fixed:
// https://github.com/Azure/azure-sdk-for-js/issues/13186
// {
// code: `import Promise from 'bluebird';${example}`,
// errors: [
// {
// message: "promises should use the in-built Promise type, not libraries or polyfills"
// }
// ]
// }

@maorleger
Copy link
Member

maorleger commented Nov 22, 2021

to @ramya-rao-a 's comment looks like "ts-package-json-sdktype", is duplicated in the ruleList. I can open a PR to fix that shortly, but I agree we should run these rules as part of CI when plugin code changes...

Edit to add: and maybe lint all the packages if any eslint plugin code changes. Haven't thought this through though...

@ramya-rao-a
Copy link
Contributor

maybe lint all the packages if any eslint plugin code changes. Haven't thought this through though...

We already have that :) See the latest on this at #18770

Copy link

Hi @deyaaeldeen, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 18, 2024
@xirzec xirzec removed this from the Backlog milestone May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eslint plugin help wanted This issue is tracking work for which community contributions would be welcomed and appreciated
Projects
None yet
Development

No branches or pull requests

4 participants