add tokenizer tests in library-declaration

[vanyauhalin]

0

Just a reminder: the ESLint sorting rule is disabled in this project as it does not always sort effectively. You can manually control the order of imports using the VSCode Sort Lines Ascending command (editor.action.sortLinesAscending) (#L5-L19).

1

Avoid meaningless comments. Instead, provide a detailed description when writing a test case. This will make your comments redundant (#L21, #L29, and so on).

For example, the "creates a union type token array" (#L30) can be rephrased to "creates tokens for the union type with the any type". The "creates a union type array with multiple different types" (#49) to "creates tokens for the union type with the any type and function type".

2

Please import types separately from other exports (#L38):

import type {Token} from "@onlyoffice/declaration-tokenizer"
import * as tokenizer from "@onlyoffice/declaration-tokenizer"
// then
const r: Token[] = []

3

In this codebase, it is uncommon to call a function within another function's argument. Instead of const r = second(first()), do:

const f = first()
const s = second(f)

4

The library-declaration package is built around factory functions with composition in mind. For each Library.Type, create a new Library.TypeNode and avoid reusing one Library.TypeNode for multiple Library.Type's. This might seem odd, but it is current practice. In the future, this may be changed.

let n = library.typeNode()
let t = library.stringType(n)

n = library.typeNode()
t = library.numberType(n)

// and so on

5

Avoid using structuredClone unless you really can not do without it. In yout test cases, you can recreate the target function's input and output using factory functions (#L49-L72).

test("creates tokens for the union type with the any type and function one", () => {
  const un = library.typeNode()
  const ut = library.unionType(un)

  let n = library.typeNode()
  let t = library.anyType(n)
  ut.push(t)

  n = library.typeNode()
  t = library.functionType(n)
  ut.push(t)

  // Often I use the letter "a" in small test cases as a short form of the word "actual"...
  const a = unionType(ut)

  // ... and the letter "e" as a short form of the word "expects".
  const e: Token[] = []

  n = tokenizer.tokenNode()
  t = tokenizer.textToken(n)
  t.text = "any"
  e.push(t)

  n = tokenizer.tokenNode()
  t = tokenizer.textToken(n)
  t.text = " | "
  e.push(t)

  n = tokenizer.tokenNode()
  t = tokenizer.textToken(n)
  t.text = "("
  e.push(t)

  n = tokenizer.tokenNode()
  t = tokenizer.textToken(n)
  t.text = ")"
  e.push(t)

  eq(a, e)
})

6

To avoid repetitive code when constructing objects, consider creating local factory functions for these test cases.

// Rename the import to avoid name collisions.
// ldt: l - library, d - declaration, t - tokenizer.
import * as ldt from "./tokenizer.ts"

// test cases are here
test.run()

function leftCurlyToken() {
  const n = tokenizer.tokenNode()
  const t = tokenizer.textToken(n)
  t.text = "("
  return t
}

function anyType() {
  const n = library.typeNode()
  return library.anyType(n)
}

function unionType() {
  const n = library.typeNode()
  return library.unionType(n)
}

7

For mapping functions, please create a separate test case for each branch. See examples here (#L404, #L734).

---

These comments focus on writing style. Please make these adjustments and I will then review the test cases in detail.

[vanyauhalin]

I have reviewed your request and made some edits. I encourage you to study these changes and try to incorporate the principles behind them in your future work. If you have any questions, feel free to ask. Once you are clear, please squash your commits, rebase, and force push.

0

Please ensure your imports are in the correct order.

1

When writing tests, it is best to start from the lowest level of abstraction and work your way up. You have started with the unionType function, but there are "lower-level" functions available, such as unknownType and voidType. Yes, they are not explicitly separated, but these can be within the request.

This package contains many functions that need testing, so it is helpful to add a prefix to each test case description. This makes navigating the file easier. You can see this pattern in the site-config package.

2

I found some of your tests to be overly complex and unclear in places. Let is take a closer look.

creates tokens for the empty union type

While the name is accurate, I have opted for a clearer one: creates tokens for the union type with no subtypes. You are using the the is function to check if the array is empty, but it would be more appropriate to use the eq. This is not critical in this case, but it is worth noting.

creates tokens for the union type with the any type

In the test description, you mention the any type. However, this test does not really check how the any type is processed within the union type. Instead, it checks how the union type creates tokens based on having one subtype. The union type must not depend on the any type. So, you are describing one thing but testing another.

• creates tokens for the union type with one subtype

creates tokens for the union type with the any type and function type

This is a more complex case with several points to consider.

When writing tests for any function, it is assumed that you start with basic operations. You have tested how the union type behaves without subtypes, then with one, and then with several. This progression from simple to complex.

However, in this case, you mention more specific cases in the description: the any type and the function type. The union type does not depend on the first one, as I mentioned earlier. It does depend on the second one, which is an edge case that should be described after checking the basic ones.

So, I see an inappropriate emphasis on the any type, a check for how the union type behaves with multiple subtypes, and a check for one edge case.

It would be better to describe each of these cases separately:

• creates tokens for the union type with multiple subtypes
• creates tokens for the union type with one function subtype
• creates tokens for the union type with the function subtype ahead
• creates tokens for the union type with the function subtype behind

You will notice that I have identified not just 2 test cases, but 4. I have moved from simple to complex: first, I check how the function type behaves when there are only one; then I check the edge cases, of which there are not just 1, as in your case, but at least 2.

creates tokens for the union type with the union type inside

This test case is correct, but it should have been preceded by a simpler case:

• creates tokens for the union type with one union subtype
• creates tokens for the union type with the union subtype

You are also using different types: any, object, void. This does not make much sense, since the behavior of the union type does not depend on them, but it does complicate the understanding of the test a little.

creates tokens for the literal type

I have chosen a more expressive name and added an additional, simpler test:

• creates tokens for the literal type with an empty value
• creates tokens for the literal type with a non-empty value

creates tokens for the empty function type

I have chosen a more expressive name: creates tokens for the function type with no parameters, returns.

creates tokens for the function type with any type parameter and void return \ creates tokens for the function type with unnamed any type parameters

Again, we have a situation where you specify specific types in the test description, but the behavior of the function type does not depend on them. I expected different kinds of tests:

• creates tokens for the function type with a parameter with an empty identifier
• creates tokens for the function type with one parameter
• creates tokens for the function type with multiple parameters
• creates tokens for the function type with a return type

creates tokens for the function type with any type and function type(with returning) parameters \ creates tokens for the function type returning function type with returning

I am not sure why these are considered edge cases.

creates tokens for the empty array type \ creates tokent for the array type with the any type item \ creates tokens for the array type with function item with returning \ creates tokens for the array type with the any type array item

I have chosen a more expressive name and removed type mentions:

• creates tokens for the array type with no items
• creates tokens for the array type with one item
• creates tokens for the array type with a function item
• creates tokens for the array type with a array item

I noticed that there are not tests covering edge cases when the array type includes the union type:

• creates tokens for the array type with a union item with one subtype
• creates tokens for the array type with a union item with multiple subtypes

creates tokens for the any type via type function \ creates tokens for the array type via type function \ creates tokens for the function via type function \ creates tokens for the literal type via type function \ creates tokens for the object type via type function \ creates tokens for the passthrough type via type function \ creates tokens for the union type via type function \ creates tokens for the unknown type via type function \ creates tokens for the void type via type function \ creates tokens for the reference via type function

The following tests use the is. However, we could use the eq, which would be a more accurate option:

• creates tokens for the reference with no id
• creates tokens for the reference with an id
• creates tokens for the any type
• creates tokens for the array type
• creates tokens for the function type
• creates tokens for the literal type
• creates tokens for the object type
• creates tokens for the passthrough type
• creates tokens for the union type
• creates tokens for the unknown type
• creates tokens for the void type

In the last test case, you used the as keyword. Please reconsider its usage as it essentially breaks typing and can often be avoided.

creates default type function tokens

I have chosen a more expressive name: creates an empty array for an unsupported type.

creates tokens for the value with unnamed any type \ creates tokens for the value with 'x' identifier and 'null' default

A similar situation, where the test case description contains irrelevant data that does not affect the value function, mixes multiple test cases into one:

• creates tokens for the value with an empty identifier
• creates tokens for the value with an identifier
• creates tokens for the value with a default value

creates tokens for the value of 'x' function with any returning

I am not sure why this is considered an edge case.

creates tokens for the unnamed any type typeDeclaration \ creates tokens for the typeDeclaration with reference type and 'X' identifier

Similarly, irrelevant data in the test case description, and two test cases are being merged:

• creates tokens for the type declaration with no identifier
• creates tokens for the type declaration with an identifier
• creates tokens for the type declaration with a reference

... and the list goes on.

3

You have done a great job moving some of the repeating constructs into helper functions. However, there are still many repetitions within the tests that could be separated. This gives the impression of a somewhat incomplete solution.

Regarding the function names, I have misunderstandings. You are using the anyTypeToken, but then the unionToken; the voidTypeToken, but then the arrayToken. I am curious, why do some type functions carry the Type suffix, while others do not?

4

When writing tests, you probably relied on the main logic. While the step-by-step method works well for the second one, given it encompasses conditions and loops, it does not quite fit the bill for test writing. These tests do not involve these parts, making this approach less suitable. So, it is better to encapsulate object creation within more abstract functions. This makes tests neater and enhances their readability.