Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completion Tests: Add script to owner #89884

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

HolonProduction
Copy link
Member

@HolonProduction HolonProduction commented Mar 25, 2024

While creating tests for the argument option completion the following problem came to light:
It is impossible to load the test scripts in a scene file, since they are not valid. That means, that the base value for completion does not reflect the members of the script.

To solve that I choose the following solution:
It is now a requirement, that the test scripts are valid, after the line which contains the indicator is removed. The completion test runner will construct a valid script under this assumption and automatically add it to the owner in the scene. This replicates the real situation when you have a valid script and start typing new code which gets auto completion.

All existing tests were adjusted to meet this criteria. I currently know of one PR which adds completion tests: #89382. If this PR is merged before this one I'll adjust the tests as well.

Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed during the GDScript meeting. This seems a fine way to fix the issue. Thanks!

Copy link
Member

@vnen vnen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Apr 23, 2024
@akien-mga akien-mga merged commit 6b28cb6 into godotengine:master Apr 23, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@HolonProduction HolonProduction deleted the tests-batch-2 branch October 5, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants