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

Organized tests into subfolders #2822

Merged
merged 15 commits into from
Aug 12, 2023
Merged

Organized tests into subfolders #2822

merged 15 commits into from
Aug 12, 2023

Conversation

AsparagusEduardo
Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo commented Mar 12, 2023 •
edited
Loading

Description

image image

Also, split into files:

  • Primal Weather
    • Desolate Land
    • Primordial Sea
  • Powder Moves
    • Overcoat
    • Safety Goggles
    • Grass types
  • Form change
    • Battle Switch
    • Begin Battle
    • Faint
  • Status1
    • Bad Poison
    • Burn
    • Freeze
    • Frostbite
    • Paralysis
    • Poison
    • Sleep

Discord contact info

AsparagusEduardo

@AsparagusEduardo AsparagusEduardo added category: battle-mechanic Pertains to battle mechanics type: cleanup category: battle-tests Related to the automated test environment labels Mar 12, 2023
@mrgriffin
Copy link
Collaborator

mrgriffin commented Mar 12, 2023 •
edited
Loading

idk if I like moving the header files from test to include/test, but if you do that, you don't need -I test in the Makefile.

EDIT: Having folders for tests seems alright to me though :) (I open all files by name so it's not any different for me, but I could see it being more helpful for others)

@AsparagusEduardo
Copy link
Collaborator Author

AsparagusEduardo commented Mar 12, 2023 •
edited
Loading

idk if I like moving the header files from test to include/test

I needed to do that otherwise it wouldn't compile D:

@AsparagusEduardo
Copy link
Collaborator Author

By the way, I removed -I test

@AsparagusEduardo
Copy link
Collaborator Author

Conflict solved, moved new tests to their respective folders.

Ready for re-review.

@AsparagusEduardo AsparagusEduardo changed the base branch from upcoming to master August 12, 2023 01:24
@AsparagusEduardo
Copy link
Collaborator Author

Solved conflicts and rebased to master

mrgriffin
mrgriffin reviewed Aug 12, 2023
View reviewed changes
Copy link
Collaborator

@mrgriffin mrgriffin left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I have one question:

  • How come we're not #include "global.h"ing in some of the tests anymore?

@AsparagusEduardo
Copy link
Collaborator Author

  • How come we're not #include "global.h"ing in some of the tests anymore?

It's included in tests/battle.h :)
I can make it so that all tests have only this one include or that they still have globah, whatever you prefer.

@mrgriffin
Copy link
Collaborator

It's included in tests/battle.h :)
I can make it so that all tests have only this one include or that they still have globah, whatever you prefer.

Ah I see! Yeah, I think I would prefer to keep the include of global.h in all the files to be consistent with regular sources, but I'll admit I don't have any real argument for doing so :)

@AsparagusEduardo
Copy link
Collaborator Author

Ah I see! Yeah, I think I would prefer to keep the include of global.h in all the files to be consistent with regular sources, but I'll admit I don't have any real argument for doing so :)

Sure, I have no issue keeping global.h there, so I'll re-add it.

@mrgriffin mrgriffin merged commit d62c0d6 into rh-hideout:master Aug 12, 2023
@AsparagusEduardo AsparagusEduardo deleted the RHH/pr/feature/testFolders branch August 12, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrgriffin mrgriffin mrgriffin approved these changes

Assignees
No one assigned
Labels
category: battle-mechanic Pertains to battle mechanics category: battle-tests Related to the automated test environment type: cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AsparagusEduardo @mrgriffin