robotcodedev / robotcode

RobotFramework support for Visual Studio Code
https://robotcode.io
Apache License 2.0
168 stars 13 forks source link

[BUG] robotcode.namespace(VariableNotFound) when using Set (Test/Suite/Global) Variable keywords #233

Open kstine opened 4 months ago

kstine commented 4 months ago

Describe the bug Whenever Setting a new variable with the Set (Test/Suite/Global) Variable keyword there is a robotcode.namespace(VariableNotFound) error message even though running the test case raises no such error.

To Reproduce

A More Complex Test Case
    [Documentation]    Test documentation
    [Tags]    standard-example-tag
    Set Test Variable    ${EXPECTED_LENGTH}    3
    ${IS_LARGER_THAN_FOUR}    Create List    suite    scope    variable
    Length Should Be    ${IS_LARGER_THAN_FOUR}    ${EXPECTED_LENGTH}

Expected behavior Expect it to detect that the variable was created previous to usage.

Screenshots/ Videos

Screenshot 2024-03-03 at 11 28 29 AM

Logs Copy the messages from VSCode "Output" view for RobotCode and RobotCode Language Server for the specific folder/workspace.

Desktop (please complete the following information):

Additional context If the variable is first listed in the Variables section then the error goes away.

d-biehl commented 4 months ago

Hi Kelby,

this works as expected, for an explanation of it see #220

also Pekka mentioned last week in his talk that it is a good style to define a default value for test/suite/global variables before using Set Test/Suite/Global Variable keyword.

Maybe this is a topic for the style guide?

kstine commented 4 months ago

From my perspective I am implementing the Set Keywords as described for RF 6.1.1 and we did publish the style guide section on variables and the tactic of what I would consider double declaring a variable seems suspect.

Even the guides page often declares variables with a value using Set (Test/Suite/Global) Variable Keywords https://docs.robotframework.org/docs/variables

I will review Pekka’s keynote, but there is quite a bit of precedent of using those Set keywords as I have given.

my work around is to disable robotcode on those lines that raise that error.

gohierf commented 4 months ago

Well, it might be good practice to define the test-scope variable in the *** Variables *** section, as discussed in #220, but this was a clever and useful workaround for variables defined in a keyword and used in another. Here we are actually in the same test case body. It is actually quite confusing.

Moreover, I would like to underline that using the equivalent VAR syntax does not raise the warning: VAR ${EXPECTED_LENGTH} 3 scope=TEST

image

My take is that there should at least be a setting to globally deactivate this warning, rather than having "to disable robotcode on those lines that raise that error".

d-biehl commented 4 months ago

@gohierf Last night I implemented that if you use a "Set Suite/Test/Task/Global Variable" in a test case or keyword, the variable is visible in the current block, but not outside the block, so it behaves like the new VAR syntax. it need some more testing but I think it is fixed in the next release. As allready mentioned in #220 it is not possible for the analyzer to say that a variable setted in one test case is visible in another test case, because the analyzer cannot say that the test case is executed before the other test case. There is no direct connection between two test cases and you can execute also only one test case.

take this example:

*** Test Cases ***
dummy
    IF    $some_condition
        Set Test Variable    ${var1}    1
    ELSE
        Set Test Variable    ${var2}    2
    END

    Should Be Equal    ${var1}    1
    Should Be Equal    ${var2}    2

because of the dynamic behavior of robot it is not possible for the static analyzer to decide wich variable is defined, and if you are looking with your eyes to this, you can also not really know what maybe happens in the test. but what you can see is, that the test maybe fails with a Variable ${*} not found and that is not that what you want at the end of the test. So defining the variables at suite level before the test runs with a default value is I think a better solution.

and btw. if the variable is used only in the same block and this block is a test case or task, there is no need to call Set Test/Task Variable or with the new syntax the use of scope=LOCAL, then a simple Set Variable or a VAR without sope is enough.

gohierf commented 4 months ago

and btw. if the variable is used only in the same block and this block is a test case or task, there is no need to call Set Test/Task Variable or with the new syntax the use of scope=LOCAL, then a simple Set Variable or a VAR without sope is enough.

In my team, we have agreed to use a test scope ${ELEMENT} variable, which is produced by "getter" keywords in our web pages. One property of the ${ELEMENT} is return/used in the keyword, but the ${ELEMENT} is also made available in test scope, for other keyword to use it.

For instance:

*** Variables ***
${ELEMENT}    SHOULD BE SET BY GETTER KEYWORD    # TEST scope

*** Keywords ***
WebUI Login: Get Username
    [Documentation]    Returns username field text (from Login page)
    ...    Sets ${ELEMENT} to username field
    ${username field}=    Get Element By    PlaceHolder    /(Enter username|Enter name)/gi
    ${username text}=    Get Text    ${username field}
    VAR    ${ELEMENT}    ${username field}    scope=TEST
    RETURN    ${username text}

WebUI Login: Set Username
    [Documentation]    Sets and returns username field text (from Login page)
    ...    Fails if readback value does not match.
    [Arguments]    ${username}=no_set_value
    WebUI Login: Get Username
    Type Text    ${ELEMENT}    ${username}
    ${username text}=    WebUI Login: Get Username
    Should Be Equal As Strings    ${username text}    ${username}
    ...    msg=Username text was not set to requested value.
    RETURN    ${username text}