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.

Sign up for GitHub

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

Jump to bottom

GD-414: Fix unclear spy test on costructor with enum parameter #415

Merged
merged 1 commit into from
Apr 25, 2024
Merged

GD-414: Fix unclear spy test on costructor with enum parameter #415

merged 1 commit into from
Apr 25, 2024

Conversation

Vacui
Copy link
Contributor

@Vacui Vacui commented Apr 25, 2024

Why

#414

What

Removed classes unique of another implementation and modified the test to fit the GdUnit4 framework.

MikeSchulze
MikeSchulze requested changes Apr 25, 2024
View reviewed changes
Copy link
Owner

@MikeSchulze MikeSchulze left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for contributing to this project.

The renaming makes sense, yes.
But have you run the test? I would say it will fail.

addons/gdUnit4/test/spy/UnitPreset.gd Outdated Show resolved Hide resolved
addons/gdUnit4/test/spy/GdUnitSpyTest.gd Outdated Show resolved Hide resolved
addons/gdUnit4/test/spy/GdUnitSpyTest.gd Show resolved Hide resolved
@MikeSchulze MikeSchulze linked an issue Apr 25, 2024 that may be closed by this pull request
GD-414: Spy test on enum constructor is unclear #414
Closed
@Vacui Vacui mentioned this pull request Apr 25, 2024
Closed
@Vacui
GD-414: Fix unclear spy test on costructor with enum parameter
5737b75 
# Why
#414

# What
Removed classes unique of another implementation and modified the test to fit the GdUnit4 framework.
@MikeSchulze MikeSchulze merged commit 3f025c7 into MikeSchulze:master Apr 25, 2024
@MikeSchulze
Copy link
Owner

Thank you 👍

@MikeSchulze
Copy link
Owner

Hm have you tested your change?
The test is failing, it has issues with the enum name.
I will push a hotfix.

@Vacui
Copy link
Contributor Author

Vacui commented Apr 25, 2024

Thanks for contributing to this project.

The renaming makes sense, yes. But have you run the test? I would say it will fail.

To be honest no. I coded everything raw on my IDE, and I was arrogant in believing everything was correct 😅
Now that I coded inside the Engine I spot every error in syntax, but I could not run the unit tests because I do not have a valid .NET setup.

@MikeSchulze
Copy link
Owner

To be honest no. I coded everything raw on my IDE, and I was arrogant in believing everything was correct 😅
Now that I coded inside the Engine I spot every error in syntax, but I could not run the unit tests because I do not have a valid .NET setup.

You don't need a .net project to run the test, but I guess you are working with the VS Code IDE right?
Next time please just run inside the Godot Editor ;)

@Vacui Vacui deleted the GD-414 branch April 25, 2024 14:36
@MikeSchulze MikeSchulze added the refactoring Mark to refactoring a implementation label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MikeSchulze MikeSchulze MikeSchulze approved these changes

Assignees

@Vacui Vacui

Labels
refactoring Mark to refactoring a implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GD-414: Spy test on enum constructor is unclear
2 participants
@Vacui @MikeSchulze