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

Unrecognised tokens in step definition in php (Behat) files #223

Open
MolbioUnige opened this issue Jul 10, 2024 · 7 comments
Open

Unrecognised tokens in step definition in php (Behat) files #223

MolbioUnige opened this issue Jul 10, 2024 · 7 comments
Labels
πŸ› bug Defect / Bug

Comments

@MolbioUnige
Copy link

MolbioUnige commented Jul 10, 2024

πŸ‘“ What did you see?

LSP don't recognize context having token defined. This is a scenario I have:

      Scenario: User is an committee member
          Given user is an committee
          And I authenticate
          Then I get redirected to "/committee"     β–  Undefined step: I get redirected to "/committee"

And here the related contexts:

      /**
       * @Given user is an committee
       */
      public function userIsAnCommittee()
      {
          throw new PendingException();
      }
      /**
       * @Given I authenticate
       */
      public function iAuthenticate()
      {
          throw new PendingException();
      }
      /**
       * @Then I get redirected to :arg1
       */
     public function iGetRedirectedTo($arg1)
      {
          throw new PendingException();
      }

βœ… What did you expect to see?

The first two contexts are recognised by the lsp and I can jump to their definition, but not the third one. I tried without the surrounding double quotes but it has the same effect.

πŸ“¦ Which tool/library version are you using?

Cucumber Language Server 1.6.0

πŸ”¬ How could we reproduce it?

No response

πŸ“š Any additional context?

No response

@kieran-ryan kieran-ryan added the πŸ› bug Defect / Bug label Jul 12, 2024
@kieran-ryan kieran-ryan transferred this issue from cucumber/language-server Jul 12, 2024
@kieran-ryan
Copy link
Member

kieran-ryan commented Jul 12, 2024

Thanks for catching this @MolbioUnige - moved to the Cucumber Language Service project ⭐

@nirajacharya2 and @thomas-hiron, you've both recently made fantastic contributions on the php implementation; wonder whether something you could look at whether you can reproduce and additionally whether you could look at what might be a suitable fix?

@thomas-hiron
Copy link

thomas-hiron commented Jul 15, 2024

Hello, I added a failing test case here: thomas-hiron@7582299
This looks to be caused by the preceding token not compatible with Ecmascript regex processor: https://regex101.com/r/7K1HfA/1

As for @MolbioUnige issue, it looks different as :arg1 doesn't match "/commitee".
This is a Gherkin/Behat (I think) functionality to match greedy data and pass that to $arg1 variable.

When I do custom steps with variables, I write them like Behat contexts:

    /**
     * @Given /^(?:|I )am on "(?P<page>[^"]+)"$/
     */
    public function visit($page)
    {
        $this->visitPath($page);
    }

I'll check if ?P<var> can be stripped before being processed.
Or maybe you see another lead @kieran-ryan?

Edit
Here's an attempt to fix it, but I got other issues: thomas-hiron@14fd2f7
I'm investigating.

@MolbioUnige
Copy link
Author

@thomas-hiron you can not add a test case in stepdefinition like that. For that, you'd need to change ExpressionBuilder.test.ts as well. My understanding is that this test file expects only two steps, with precise definition, the two that are defined actually.

Behat is the only php framework I know for Gherkin files. How do you test your feature files?

@thomas-hiron
Copy link

Yes, that was not meant to be merged, but to highlight the fact that ES regex processor doesn't handle Behat default regex!
When I fixed the test with thomas-hiron@14fd2f7, the test was still failing due to another expected output as you mentioned!

I do test my feature files with Behat also!

For your precise issue, maybe the language service should replace :[a-z0-9]+ by the regex itself?

.replace(/:[a-z0-9]+/, ':[a-z0-9]+')

This should transform your :arg1 in an understandable regex for the language service?

@MolbioUnige
Copy link
Author

See my PR

@MolbioUnige
Copy link
Author

Dear maintainers, is there anything else I need to do for my PR to be taken in account?

@develth
Copy link

develth commented Aug 26, 2024

would be great that get this work or to provide another way to achieve that.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
πŸ› bug Defect / Bug
Projects
None yet
Development

No branches or pull requests

4 participants