Tools: 鈽傦笍 Missing Tests

Created on 10 Aug 2020  路  26Comments  路  Source: rome/tools

List of all missing tests that need to be done.

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

good first issue tests umbrella

Most helpful comment

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.

All 26 comments

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

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

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

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

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.

@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!

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",
);

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

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

They've been removed.

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

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

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

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.

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

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

will try js-ast-utils/getJsxElementName as well

I'd like to take

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

@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! https://github.com/romefrontend/rome/pull/1138

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

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

I'll try js-ast-utils/valueToNode.ts

Removed @JustBeYou from parser-core, and @ia17011 from js-ast-utils/isTypeNode.ts as there haven't been any updates for a few weeks.
If you want to take them back just post a comment.

I'd like to work on js-ast-utils/isTypeNode.ts

I'd like to work on js-ast-utils/isTypeNode.ts

@Naturalclar A test for js-ast-utils/isTypeNode.ts is no longer needed as it's covered by the test for js-ast-utils/isInTypeAnnotation.

But thanks for wanting to help!

@jer3m01 got it, thanks for the info!

was looking for good first issues and came across this which seems completed (if the checklist is accurate). Maybe worth closing or updating with what additional code needs testing. I imagine now, given the velocity of the project, that tests are requirements to get something merged and so there may not be anymore missing tests

Yes, thanks!

We should go through issues and add good first issues label if they are. A lot will be added once #1298 is complete.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  路  5Comments

sebmck picture sebmck  路  4Comments

sebmck picture sebmck  路  3Comments

sebmck picture sebmck  路  3Comments

Kelbie picture Kelbie  路  4Comments