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

Erroneous test result. #285

Closed
Lord-Kamina opened this issue Oct 21, 2022 · 3 comments
Closed

Erroneous test result. #285

Lord-Kamina opened this issue Oct 21, 2022 · 3 comments

Comments

@Lord-Kamina
Copy link
Contributor

In master, you have the following:

[
    "\\/:pre?baz",
    undefined,
    [
      "/",
      {
        name: "pre",
        prefix: "",
        suffix: "",
        modifier: "?",
        pattern: "[^\\/#\\?]+?",
      },
      "baz",
    ],
    [
      ["/foobaz", ["/foobaz", "foo"]],
      ["/baz", ["/baz", undefined]],
    ]

But this is wrong. In reality, /baz would not match because options.start defaults to true.

I'm not entirely sure but I think the erroneous test result might be related to an incorrect tokenization in the test definition. With the code in master, the tokens actually look like the following:

[
    "\\/:pre?baz",
    undefined,
    [
-      "/",
      {
        name: "pre",
-      prefix: "",
+      prefix: "/",
        suffix: "",
        modifier: "?",
        pattern: "[^\\/#\\?]+?",
      },
      "baz",
    ],
    [
      ["/foobaz", ["/foobaz", "foo"]],
      ["/baz", ["/baz", undefined]],
    ]
@blakeembrey
Copy link
Member

The code on master is passing the test. I have it passing locally too. I’m not sure what you’re referring to when you say the “code on master” has a different result.

@blakeembrey
Copy link
Member

I'm finding it hard to discuss "in reality" here, because in reality these tests pass and /baz does match. Also, you're mentioning two things:

In reality, /baz would not match because options.start defaults to true.

In reality it does match and that's a code functionality of this library, to match this code.

With the code in master, the tokens actually look like the following

Are you saying, that in master, with zero code modifications, the test suite is failing? Can you share something to confirm this?

@Lord-Kamina
Copy link
Contributor Author

Jesus ignore me; I was very tired.

What I thought was happening was parse() returned a different value than what the test file actually registered (for me, both running your code as well as the ported version I was getting the same output, two tokens, one of them with a prefix) but I just realized I hadn't escaped the / in my route. Ignore this.

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

No branches or pull requests

2 participants