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

☂️ Missing Tests #1023

Open
jer3m01 opened this issue Aug 10, 2020 · 19 comments
Open

☂️ Missing Tests #1023

jer3m01 opened this issue Aug 10, 2020 · 19 comments

Comments

@jer3m01
Copy link
Contributor

@jer3m01 jer3m01 commented Aug 10, 2020

List of all missing tests that need to be done.

  • ast-utils/removeLoc.ts
  • codec-js-regexp/index.ts
  • codec-source-map/ArraySet.ts @nt591
  • codec-source-map/base64.ts
  • codec-source-map/MappingList.ts
  • codec-source-map/SourceMapConsumer.ts
  • codec-source-map/SourceMapConsumerCollection.ts
  • codec-source-map/SourceMapGenerator.ts
  • js-ast-utils/assertMultipleNodes.ts @aravind1078
  • js-ast-utils/assertSingleNode.ts @aravind1078
  • js-ast-utils/assertSingleOrMultipleNodes.ts @aravind1078
  • js-ast-utils/cleanJSXText.ts
  • js-ast-utils/createMemberProperty.ts
  • js-ast-utils/createPropertyKey.ts
  • js-ast-utils/getBindingIdentifiers.ts
  • js-ast-utils/getImportSpecifiers.ts
  • js-ast-utils/getJSXAttribute.ts
  • js-ast-utils/getJSXElementName.ts @aravind1078
  • js-ast-utils/getTSQualifiedBaseFromEntityName.ts
  • js-ast-utils/hasJSXAttribute.ts
  • js-ast-utils/hasPotentialSideEffects.ts
  • js-ast-utils/inheritLoc.ts
  • js-ast-utils/isBinary.ts @nt591
  • js-ast-utils/isEmptyTemplateLiteral.ts
  • js-ast-utils/isFunctionNode.ts @aravind1078
  • js-ast-utils/isInTypeAnnotation.ts
  • js-ast-utils/isJSXElement.ts @aravind1078
  • js-ast-utils/isNodeLike.ts @sasurau4
  • js-ast-utils/isTypeNode.ts ⚙️ @ia17011
  • js-ast-utils/isValidIdentifierName.ts
  • js-ast-utils/removeShallowLoc.ts
  • js-ast-utils/renameBindings.ts
  • js-ast-utils/resolveIndirection.ts
  • js-ast-utils/template.ts
  • js-ast-utils/tryStaticEvaluation.ts @nt591
  • js-ast-utils/tryStaticEvaluationPath.ts @nt591
  • js-ast-utils/valueToNode.ts
  • markup-syntax-hightlight/index.ts @JustBeYou
  • parser-core/comments.ts ⚙️ @JustBeYou
  • parser-core/ParserCore.ts ⚙️ @JustBeYou
  • parser-core/utils.ts ⚙️ @JustBeYou
  • path-match/stringify.ts @JustBeYou
  • string-diff/index.ts ⚙️ @JustBeYou
  • vcs/index.ts @JustBeYou

If any of them are not worth having test or some are missing, please comment.

A ⚙️ indicates they are being worked on.
A checkmark indicates an open PR.

@jer3m01 jer3m01 added the umbrella label Aug 10, 2020
@diokey diokey added the tests label Aug 12, 2020
@nt591
Copy link
Contributor

@nt591 nt591 commented Aug 12, 2020

I'm going to look into js-ast-utils/isBinary.ts now

@nt591
Copy link
Contributor

@nt591 nt591 commented Aug 13, 2020

I'm adding tests for js-ast-utils/isFor

@nt591
Copy link
Contributor

@nt591 nt591 commented Aug 13, 2020

I'm also adding tests for codec-source-map/ArraySet.ts

Should I start collapsing these posts into one and edit accordingly?

@sebmck
Copy link
Member

@sebmck sebmck commented Aug 13, 2020

Sorry I didn't have time to review the list of tests in the issue body. We don't need tests for basic methods that are just refining types, they are extremely excessive and will be implicitly integration tested elsehwere.

@sebmck
Copy link
Member

@sebmck sebmck commented Aug 13, 2020

@nt591 Feel free to batch them up into a single PR, whatever's easiest. If you're only adding new files then you shouldn't need to worry about merge conflcits at all! Thank you for doing this work!

@sebmck
Copy link
Member

@sebmck sebmck commented Aug 13, 2020

Also when implementing any tests that operate on an AST, preferably inline the AST itself by manually constructing it with the create builder methods in @internal/ast, and if you really need to parse JS (such as a large example), use:

import {parseJS} from "@internal/js-parser";

parseJS({
  input: "CODE HERE",
  path: "unknown",
);
@hanhanhan
Copy link
Contributor

@hanhanhan hanhanhan commented Aug 14, 2020

@nt591 and I started in on js-ast-utils/isUnaryLike.ts

Does the strikethrough mean it's one of the unneeded "basic methods that are just refining types" to be tested elsewhere?

We noticed throw is a JSThrowStatement not a UnaryExpression.

@jer3m01
Copy link
Contributor Author

@jer3m01 jer3m01 commented Aug 14, 2020

Yes the strikethrough means a test for it isn't needed.
I'll add that the the description.

They've been removed.

@aravind1078
Copy link
Contributor

@aravind1078 aravind1078 commented Aug 14, 2020

i'm going to look into js-ast-utils/isJsxElement

@aravind1078
Copy link
Contributor

@aravind1078 aravind1078 commented Aug 15, 2020

I will take
js-ast-utils/assertMultipleNodes.ts
js-ast-utils/assertSingleNode.ts
js-ast-utils/assertSingleOrMultipleNodes.ts
and try to batch them together bcz all of them looks fairly similar

@sebmck
Copy link
Member

@sebmck sebmck commented Aug 15, 2020

@hanhanhan

We noticed throw is a JSThrowStatement not a UnaryExpression.

Oh that's really bizzare, that shouldn't be there... Want to open a PR to remove it? TypeScript checks should be enough to validate it's not used anywhere.

@sebmck
Copy link
Member

@sebmck sebmck commented Aug 15, 2020

Thank you so much everyone for the PRs! I really appreciate it, especially since you've all written them without any context. I've long neglected tests for most of the code I've written since it's been a colossal effort to build it in the first place.

@JustBeYou
Copy link
Contributor

@JustBeYou JustBeYou commented Aug 17, 2020

I think I will give a try to the things under js-ast-utils:

  • markup-syntax-hightlight/index.ts
  • parser-core/comments.ts
  • parser-core/ParserCore.ts
  • parser-core/utils.ts
  • string-diff/index.ts
  • vcs/index.ts
@aravind1078
Copy link
Contributor

@aravind1078 aravind1078 commented Aug 18, 2020

i will give a try at js-ast-utils/isFunctionNode

@aravind1078
Copy link
Contributor

@aravind1078 aravind1078 commented Aug 18, 2020

will try js-ast-utils/getJsxElementName as well

@nt591
Copy link
Contributor

@nt591 nt591 commented Aug 18, 2020

I'd like to take

js-ast-utils/tryStaticEvaluation.ts
js-ast-utils/tryStaticEvaluationPath.ts

sebmck pushed a commit that referenced this issue Aug 19, 2020
@hanhanhan
Copy link
Contributor

@hanhanhan hanhanhan commented Aug 21, 2020

@hanhanhan

We noticed throw is a JSThrowStatement not a UnaryExpression.

Oh that's really bizzare, that shouldn't be there... Want to open a PR to remove it? TypeScript checks should be enough to validate it's not used anywhere.

Here it is! #1138

@sasurau4
Copy link
Contributor

@sasurau4 sasurau4 commented Aug 26, 2020

I'll work on js-ast-utils/isNodeLike.ts.

@ia17011
Copy link

@ia17011 ia17011 commented Sep 25, 2020

I'll take js-ast-utils/isTypeNode.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.