renandelmonico / vscode-phpunit

The VS Code Test Explorer extension for PHPUnit
9 stars 2 forks source link

plugin inject wrong config file #5

Closed tyler36 closed 1 year ago

tyler36 commented 2 years ago

I have a DDEV project I run in Remote WSL session.

If I configure this plugin to run with the following settings

    "phpunit.relativeFilePath": true,
    "phpunit.phpunit": "/var/www/html/vendor/bin/phpunit",
    "phpunit.docker": true,
    "phpunit.dockerImage": "docker exec -it 833f9",

I get the following error:

spawn docker exec -it 833f9 bash -c "var/www/html/vendor/bin/phpunit -c /home/user13/code/phpunit.xml" ENOENT

The following is successful

I do not understand why -c /home/user13/code/phpunit.xml is being injected. This config file is my WSL path. If the command is running in docker, then it should be the dockered path?

I check the actual path:

docker exec -it 833f9 "bash"

$ find ~+ -type f -name phpunit.xml
/var/www/html/phpunit.xml

I can run the following in my terminal:

So I think the correct spawn command should be:

VSCode 1.66.2 renandelmonico.vscode-php-test-explorer 1.0.1

renandelmonico commented 2 years ago

Hello! If the command is running in docker then should be used the dockered path. To avoid this problem I recomend to use the -v $(pwd):$(pwd) in the dockerImage config.

Sorry, but I didn't understand the problem. Did you mean that WSL path of phpunit.xml should be converted to the dockered path?

tyler36 commented 2 years ago

Did you mean that WSL path of phpunit.xml should be converted to the dockered path?

Yes.

This extension generates this error:

spawn docker exec -it 833f9 bash -c "var/www/html/vendor/bin/phpunit -c /home/user13/code/phpunit.xml" ENOENT
  1. Why is "-c /home/user13/code/phpunit.xml" added to the command?
  2. -c /home/user13/code/phpunit.xml is on WSL. It is not in docker.
renandelmonico commented 2 years ago

Did you mean that WSL path of phpunit.xml should be converted to the dockered path?

Yes.

This extension generates this error:

spawn docker exec -it 833f9 bash -c "var/www/html/vendor/bin/phpunit -c /home/user13/code/phpunit.xml" ENOENT
  1. Why is "-c /home/user13/code/phpunit.xml" added to the command?
  2. -c /home/user13/code/phpunit.xml is on WSL. It is not in docker.

I think it shouldn't add phpunit.xml if you didn't specify the phpunit.xml path in config file. Do you agree?

tyler36 commented 2 years ago

I think it shouldn't add phpunit.xml if you didn't specify the phpunit.xml path in config file. Do you agree?

Yes. I don't think it shoud add it.

Maybe you could add a config option. EG. "phpunit.config_file": "/home/user13/code/phpunit.xml"

renandelmonico commented 2 years ago

I think it shouldn't add phpunit.xml if you didn't specify the phpunit.xml path in config file. Do you agree?

Yes. I don't think it shoud add it.

Maybe you could add a config option. EG. "phpunit.config_file": "/home/user13/code/phpunit.xml"

  • if ! phpunit.config_file do nothing
  • if phpunit.config_file, add "-c /home/user13/code/phpunit.xml" to spawn command

Yes, I think it's the best way. I'll change the code soon with this bugfix.

renandelmonico commented 2 years ago

@tyler36 Can you test if the implementation in Pull Request #6 works? :smile: If works, I'll publish a new version with this bugfix.

tyler36 commented 2 years ago

@renandelmonico

I'm not sure how to apply and test this changes.

renandelmonico commented 2 years ago

@renandelmonico

I'm not sure how to apply and test this changes.

I'll launch a new version (1.0.2) and if it includes new bugs, please, tell me, because I tested but I needed more person testing :smile:

tyler36 commented 2 years ago

Thank you for the update.

Given the following config:

    "phpunit.relativeFilePath": true,
    "phpunit.phpunit": "/var/www/html/vendor/bin/phpunit",
    "phpunit.docker": true,
    "phpunit.dockerImage": "docker exec -it e1066",

It generates the following error: spawn docker exec -it e1066 bash -c "var/www/html/vendor/bin/phpunit " ENOENT

2 things:

docker exec -it e1066 "var/www/html/vendor/bin/phpunit " : "no such file or directory: unknown" ❌ docker exec -it e1066 "var/www/html/vendor/bin/phpunit": "no such file or directory: unknown" ❌ docker exec -it e1066 "/var/www/html/vendor/bin/phpunit ": "no such file or directory: unknown" ✅ docker exec -it e1066 "/var/www/html/vendor/bin/phpunit" : works fine.

renandelmonico commented 2 years ago

Sorry for the late response but I'm very busy. When I'll be free I'll see these cases.

renandelmonico commented 2 years ago

Thank you for the update.

Given the following config:

    "phpunit.relativeFilePath": true,
    "phpunit.phpunit": "/var/www/html/vendor/bin/phpunit",
    "phpunit.docker": true,
    "phpunit.dockerImage": "docker exec -it e1066",

It generates the following error: spawn docker exec -it e1066 bash -c "var/www/html/vendor/bin/phpunit " ENOENT

2 things:

  • It's misisng the / at the beginning /var/www ....
  • it addes a ` (space) afterphpunit`.

x docker exec -it e1066 "var/www/html/vendor/bin/phpunit " : "no such file or directory: unknown" x docker exec -it e1066 "var/www/html/vendor/bin/phpunit": "no such file or directory: unknown" x docker exec -it e1066 "/var/www/html/vendor/bin/phpunit ": "no such file or directory: unknown" white_check_mark docker exec -it e1066 "/var/www/html/vendor/bin/phpunit" : works fine.

I think there're 2 points in your error:

1 - If "phpunit.relativeFilePath": true the extension will remove the / at the beginning /var/www because basically you're saying that the path is relative. i think it's not a bug. 2 - The space after phpunit is a bug! :smile:

renandelmonico commented 1 year ago

Sorry for the late response. The main project has been updated. I think we should try it because there are more features. :D