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

Add proper test #922

Closed
wants to merge 2 commits into from
Closed

Conversation

touilleMan
Copy link
Member

@touilleMan touilleMan commented Nov 12, 2022

  • The current test folder has been renamed example
  • A new test folder has been created, it contains a simple project which has a GDScript file running asserts on the built extension, then closing the app

Now I'm not sure how we should run this in the CI:

  • For iOS&Android an emulator is needed, this seems overkill to me so it should be good enough to ignore those platforms for the moment
  • To run the test we must have a Godot binary, which requires 1) download the latest official build or 2) build it here

As long as Godot 4 is not released, there is frequent breakage on Godot master, so downloading the latest official build seems not viable (but on the other hand it seems the way to go when Godot 4 is released)
On the other hand, building Godot from the master seems overkill, so I'm not sure we want that.

Even if we don't add this test to Godot-CPP's, we should run it in the Godot CI where Godot-CPP is currently build.

PS: I'm not sure I'll have much time to continue work on this, so feel free to modify this PR (or even copy part of it without bothering with crediting me and create your own PR 😄 )

Copy link
Member

@groud groud left a comment

Choose a reason for hiding this comment

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

We reviewed this PR in the GDextension review meeting. We overall agree with the idea of spliting tests and examples into two different folders. Here are some feedback:

  • The example.cpp/.h files in test should probably be renamed to test.cpp/.h.
  • The example is a bit too complex now, and should limit itself to one property, and a few representative example functions.

@dsnopek
Copy link
Collaborator

dsnopek commented May 1, 2023

Superseded by #1101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants