correct logic for tests in 20.1 and provide more descriptive errors

[mstphayassin]

Please check if the PR fulfills these requirements:

• The commit message follows our guidelines.
• For bug fixes and features:
  • You tested the changes.

Related issue (if applicable): #1125 and #1169

The tests should all run appropriately now, as seen in these screenshots:

The only case I wasn't sure about was the situation described in #1125

While this may be confusing, I believe that failing the test in #1125 is not a bug and should be considered the intended functionality.

The reason for this behavior is the use of the var keyword within the scope of the run() function. When var is used, a new variable is initialized which is only defined in the scope it's initialized in. So, for example:

# This variable is defined in the global scope.
var robot_name = "INVALID NAME"

func run():
    # This variable is defined only in the run() function.
    var robot_name = "huda"
   
    # Testing what this variable's name is in the scope of the run() function
    print(robot_name) 
    # Result: huda
 

# Later, outside of the run() function

# Testing if you got the right answer:
print(robot_name)
# result: INVALID NAME

In the run() function, there is a local variable called robot_name that you initialized that only exists in run(). Meanwhile, there is a global variable, also called robot_name, that is invalid, and it stays invalid because it was never accessed or changed.

All of this to say that I don't see any other way around this other than explicitly directing users not to use var in their code here. It might be confusing to them and they might wonder why, but it's even more confusing to leave it as is.

Here's the new functionality in these cases:

[NathanLovato]

Thank you very much for the help there and for the detailed report. I've actually gone ahead and changed the practice to expose the property and I think this should be the direction for practices moving forward to expose more code because it's generally been a source of confusion for some students.

The issue I'm talking about is not specific to this practice, but more holistic and applying to the entire course. It's actually briefly documented in an issue here: #759

Originally we wanted to make to have all the code that the student would need to understand the context of the practice with just the code but there's but freeze it to avoid to prevent edits to it. In the absence of that, we went with hiding code that the student should not be able to modify instead. As you know, we can't expose all the code of the practices because there's more happening behind the scenes than the student can wrap their head around at this point.

Anyway, for this practice, it's relatively safe to expose the property so I decided to go with that.

[NathanLovato]

Thank you! Is there anything you wish was documented or explained regarding, for example, the codebase, app architecture, guidelines for practices, anything that could help you navigate the app and test or iterate on things?

[mstphayassin]

@NathanLovato That's a good question. I'm not sure. I think one thing that I'm still confused about and that I can't find much information on is the use of the #EXPORT and #/EXPORT keywords and how that influences the rest of the app. Other than that I think the code is written clearly and the codebase is organized intuitively.

[NathanLovato]

Thanks! I went a bit beyond explaining just this export code region thingy and wrote some initial docs for the practices in general: https://github.com/GDQuest/learn-gdscript/blob/main/docs/interactive_practices.md

I hope this helps. In short #EXPORT code regions delineate the practice solution. We used to have a bunch of stuff around that (stub support for a sandboxed GDScript editor, multiple files, etc.), but recently I simplified this part of the codebase to just parse those code regions when loading a practice and registering that code as the sample solution for the practice.