MarketSquare / robotframework-browser

Robot Framework Browser library powered by Playwright.
Apache License 2.0
519 stars 105 forks source link

feat: only check for node when robot is running #3758

Closed Leemur89 closed 1 day ago

Leemur89 commented 1 month ago

Hello,

Sometimes the code completion of IDE fails to import the library due to some issues with the PATH, while when running the test the lib is perfectly imported. This can happen when the IDE does not properly initiate the PATH with node, without impacting the run. This is why I am suggesting to check if node is installed only if the test is actually running, which would allow IDE to load the keywords even if node is not in PATH

aaltat commented 1 month ago

Could you fix the linting error with ruff.

Leemur89 commented 1 month ago

Hi @aaltat , I've fixed the linting error but now I got an error on the MacOS run with python 3.12

Screenshotting With Jpeg Extension And Quality Borders                  
  Traceback (most recent call last):
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    File  /Users/runner/work/robotframework-browser/robotframework-browser/atest/test/02_Content_Keywords/screenshot.robot:49
    T:  Screenshotting With Jpeg Extension And Quality Borders    
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    File  /Users/runner/work/robotframework-browser/robotframework-browser/atest/test/02_Content_Keywords/screenshot.robot:59
      Numbers Are Close    ${size_100}    ${size_101}    10
      |  ${size_100} = 89564 (int)
      |  ${size_101} = 89602 (int)
______________________________________________________________________________
| FAIL |

I don't really know what this is about sorry :/

aaltat commented 1 month ago

That screenshot test is quite flaky, but seems that reruns did help

aaltat commented 1 month ago

This is tricky thing. I understand your need and the change in the code. But we support running library also without Robot Framework and we have unit tests in place for that. Also there is documentation that such feature is supported.

Now if we make the change, check is not done in when running library without Robot Framework. The library will fail, but in different way. The question is: will that change user experience in significant way and is the easier option to configure IDE in such way that the error doesn’t appear. Example of library does not work in your IDE, does debugging test case, to the library level then work?

aaltat commented 1 month ago

@Snooz82 what you think?

Snooz82 commented 1 month ago

Hi,

No. not that way! i drafted the topic already and i would just have made a property. the problem with „robot running“ is, that this would made the usage without robot impossible.

regards

Leemur89 commented 1 month ago

Hi @aaltat ,

The problem I have is when I launch VSCode from the application list on Mac, it does not properly use my environment variables, and node is not in my PATH. Running a test is working fine, but the linter that I'm using fails to find it, hence fails to start the lib and discover the keywords

@Snooz82 would that be better if I do this skip only if robot can be imported?

aaltat commented 1 month ago

Configuring Mac with VSCode is possible, I have Mac + VSCode and it works. But that only proves that it works on my machine and I don’t have solution for your problem.

In any case, I don’t see how skipping when Robots Framework can not be imported would solve this problem? Would you care to explain why that would work?

Leemur89 commented 3 weeks ago

This is only for the linter/autocompletion to work. I'm using robotcode extension, but for some reason node is not in the path of the linter/autocompletion, which makes it fail to import the lib and discover the keywords. Running tests is OK because it is starting from a terminal which does have node in path.

In the end this skip is only to allow the discovery of the keywords, I've been also doing it for a private remote libjs library: when I edit my files, the remote lib is not running, but thanks to this condition, I'm still able to benefit from autocomplete

Leemur89 commented 3 weeks ago

@Snooz82 what would be the use case of a usage without robot? this is robotframework-browser, I'm actually surprised that this could be used without robot

aaltat commented 1 week ago

We discussed in the team and consensus was that we would skip starting the node process if there is not plugins initialized. Because one should get keywords if plugins are not present, it is just Python code, but if there are plugins, specially the node extensions, we need the node process to do the discovery.

Snooz82 commented 1 day ago

This PR should be obsolete due to a different fix in #3822