zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
35.33k stars 1.79k forks source link

php: Add runnable tests #11514

Closed RemcoSmitsDev closed 1 week ago

RemcoSmitsDev commented 3 weeks ago

This pull request adds the following:

Queries explanations

Query 1 (PHPUnit: Run specific method test):

  1. Class is not abstract (because you cannot run tests from an abstract class)
  2. Class has Test suffix
  3. Method has public modifier(or no modifiers, default is public)
  4. Method has test prefix

Query 2 (PHPUnit: Run specific method test with @test annotation):

  1. Class is not abstract (because you cannot run tests from an abstract class)
  2. Class has Test suffix
  3. Method has public modifier(or no modifiers, default is public)
  4. Method has @test annotation

Query 3 (PHPUnit: Run specific method test with #[Test] attribute):

  1. Class is not abstract (because you cannot run tests from an abstract class)
  2. Class has Test suffix
  3. Method has public modifier(or no modifiers, default is public)
  4. Method has #[Test] attribute

Query 4 (PHPUnit: Run all tests inside the class):

  1. Class is not abstract (because you cannot run tests from an abstract class)
  2. Class has Test suffix

Query 5 (Pest: Run function test)

  1. Function expression has one of the following names: describe, it or test
  2. Function expression first argument is a string

PHPUnit: Example for valid test class

Screenshot 2024-05-08 at 10 41 34

PHPUnit: Example for invalid test class

All the methods should be ignored because you cannot run tests on an abstract class.

Screenshot 2024-05-07 at 22 28 57

Pest: Example

https://github.com/zed-industries/zed/assets/62463826/bce133eb-0a6f-4ca2-9739-12d9169bb9d6

You should now see all your Pest tests inside the buffer symbols modal. Screenshot 2024-05-08 at 22 51 25

Release Notes:

RemcoSmitsDev commented 3 weeks ago

Cool! Just leaving a few comments based on a version that I was working on before I saw that someone had beat me to it! 😄

Ahhh sorry man, didn't know that. I hope this satisfies your needs!

RemcoSmitsDev commented 3 weeks ago

LGTM, thanks Remco :)

I gonna add pest & docblock annotation support aswell. Working on it right now!

claytonrcarter commented 3 weeks ago

Ahhh sorry man, didn't know that. I hope this satisfies your needs!

It definitely does. No worries here! Thank you!

RemcoSmitsDev commented 3 weeks ago

Ahhh sorry man, didn't know that. I hope this satisfies your needs!

It definitely does. No worries here! Thank you!

You want to hack on the pest support together?

RemcoSmitsDev commented 3 weeks ago

@osiewicz Can you help me out, I have the following 2 queries. They seem to work because I can see the runnable icon before the test functions. But when I click on them nothing happens, what am I missing here? Or not understanding correctly? It seems like it cannot get the value correctly from the query.

Screenshot 2024-05-08 at 16 06 07

This query support non-chaining methods:

test("example", function () {
    expect(true)->toBeTrue();
});
(
    (expression_statement
        (function_call_expression
            function: (_) @name
            (#any-of? @name "it" "test" "describe")
            arguments: (arguments
                (argument
                    (encapsed_string (string_value)) @run
                )
            )
        )
    )
) @pest-test

This query support chaining methods:

test("example", function () {
    expect(true)->toBeTrue();
})->with(["1", "2"]);
(
    (expression_statement
        (member_call_expression
            object: (function_call_expression
                function: (_) @name
                (#any-of? @name "it" "test" "describe")
                arguments: (arguments
                    (argument
                        (encapsed_string (string_value)) @run
                    )
                )
            )
        )
    )
) @pest-test
RemcoSmitsDev commented 3 weeks ago

While digging inside the runnable/task code, looking why clicking the runnable icon inside the gutter for Pest runnable didn't open a new terminal panel to run the tests in. I finally found the issue that not only applies to Pest runnable.

The issue is that the following code does not contain any symbols according to the Zed PHP outline scheme.

<?php

test("example", function () {
    expect(true)->toBeTrue();
});

describe("tests", function () {
    test("test1", function () {
        expect(true)->toBeTrue();
    });

    test("test2", function () {
        expect(true)->toBeTrue();
    });
});

it("has a name")
    ->expect(fn() => User::create(["name" => "Nuno Maduro"])->name)
    ->toBe("Nuno Maduro");

So when the task context is getting build inside task_context::build_context (line: 53). We ask for all the symbols, but this will give us an empty Vec back. Because we have an empty Vec we cannot determine what the symbol for the task context should be.

So if we go higher up the stack we are trying to build a task from the task template, which could not be done because the symbol information does not exist inside the context which is required for the Pest task template.


So I was talking to @osiewicz to explain what I found, why the Pest runnable did not work. So I came up with a fix for this issue by adding the following code to the PHP outline scheme:

; Add support for Pest runnable
(function_call_expression
    function: (_) @context
    (#any-of? @context "it" "test" "describe")
    arguments: (arguments
        (argument
            (encapsed_string (string_value) @name)
        )
    )
) @item

After adding this to the outline scheme, I got the Pest runnable too work😀 That being said, as I already mentioned we are going to have this issue with more languages. So @osiewicz came up with an idea to store the query captures inside the Env key of the task context. So we could use them instead of having to add these expressions to the outline scheme.

Screenshot 2024-05-08 at 22 51 25


Note

We have to think about how we want to solve this in the feature. For now, having them inside the outline scheme is fine.

RemcoSmitsDev commented 3 weeks ago

@osiewicz one thing I'm not happy about yet is that the Pest query/outline scheme is not specific enough. This results in invalid results that should not happen, but could.

Screenshot 2024-05-09 at 11 45 33

The original query/scheme that I made prevented this, but did not support more than one chained method. This is because the more chained methods you have, the more member_call_expressions you have as child nodes. I could not fixture out how I could write a query/scheme that has dynamic wildcard for more than one child node using the (_) syntax.

So ideally I want to wildcard all the member_call_expressions that are inside the expression_statement. Is there away to do this?

I had a query similar to this to wildcard a single child node.

(
    (expression_statement
        (_
            (function_call_expression
                function: (_) @name
                (#any-of? @name "it" "test" "describe")
                arguments: (arguments
                    (argument
                        (encapsed_string (string_value)) @run
                    )
                )
            )
        )
    )
) @pest-test
Screenshot 2024-05-09 at 11 43 43
osiewicz commented 3 weeks ago

Hey, wanna pair on it sometime later in the day? E.g. at 5 PM (we're in the same timezone).

RemcoSmitsDev commented 3 weeks ago

Yeah sure, sounds good!

claytonrcarter commented 3 weeks ago

This results in invalid results that should not happen, but could.

Can Pest tests be defined in classes, or are they only correct when defined at the "top level" of the file/script? If so, could it work to query for any function_call_expression that are not in a class_declaration?

RemcoSmitsDev commented 3 weeks ago

Pest does not support the following methods it, test and describe inside classes or function declarations. Because these functions are expressions. That way they are register as a test case. You if you would define them in any declaration kind it's not getting registered as a test case.

Yeah, we have to do something like that. But I'm not sure how to do that.

claytonrcarter commented 3 weeks ago

Pest does not support the following methods it, test and describe inside classes or function declarations

If this is the case, would it work to wrap the query in (program (expression_statement ...)) to only match expressions at the "top level" and not, like in your example, within a class method? Hmm, though I guess that wouldn't work if the tests were wrapped in if (...) { } or foreach (...) { }?

Is there a tree-sitter way to query for something that does not have a particular node as an ancestor?

RemcoSmitsDev commented 3 weeks ago

Yeah, adding the program as top level node inside the query fixes the issue that it matches also expressions inside the function/class declarations. The query looks like this:

; Match non chained methods for Pest test
(program
    (expression_statement
        (function_call_expression
            function: (_) @_name
            (#any-of? @_name "it" "test" "describe")
            arguments: (arguments
                .
                (argument
                    (encapsed_string (string_value) @run)
                )
            )
        )
    )
) @pest-test

So I expect that no one wraps them inside an if or foreach state. Because then you are using Pest totally in the wrong way. So let's not worry about that.

mansoorkhan96 commented 2 weeks ago

@RemcoSmitsDev possible to add support for Laravel Dusk?

RemcoSmitsDev commented 2 weeks ago

@mansoorkhan96 You can make that possible by your self, when this pull request is merged by adding the following task to your ~/.config/zed/tasks.json

{
  "label": "laravel dusk test $ZED_SYMBOL",
  "command": "php artisan dusk",
  "args": ["--filter $ZED_SYMBOL $ZED_FILE"],
  "tags": ["phpunit-test", "pest-test"]
}
osiewicz commented 2 weeks ago

@RemcoSmitsDev I've merged main into this branch btw

osiewicz commented 1 week ago

Thanks!