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

Tests: Added more specific error messages to the duplicate id asserts #2428

Closed

Conversation

Jarno458
Copy link
Collaborator

@Jarno458 Jarno458 commented Nov 5, 2023

What is this fixing or adding?

Added additional comparisations to the duplicate id's check to actually print out the duplicated ids

How was this tested?

ran them

@Jarno458 Jarno458 added the is: refactor/cleanup Improvements to code/output readability or organizization. label Nov 5, 2023
@alwaysintreble
Copy link
Collaborator

Every single case where you added the game name already says the relevant game in the test run.

@Jarno458
Copy link
Collaborator Author

Jarno458 commented Nov 5, 2023

i had a hard time finding it

@alwaysintreble
Copy link
Collaborator

in pycharm
Screenshot_76
on github
Screenshot_77

@Jarno458
Copy link
Collaborator Author

Jarno458 commented Nov 5, 2023

seems like my test runner doesnt display those subtests like that

@alwaysintreble
Copy link
Collaborator

@Jarno458
Copy link
Collaborator Author

Jarno458 commented Nov 5, 2023

Mine doesn't allow me to go to that level, it does still run the test for all games 1 by 1 just fine tho
image

@Berserker66
Copy link
Member

get a better test runner? No idea which one that even is.

@Jarno458
Copy link
Collaborator Author

Jarno458 commented Nov 6, 2023

I dont think adding those game names hurt, might help people who use subpar test runners
but if am required to remove them again i can, let me know if its required to remove them

@ThePhar ThePhar added the affects: core Issues/PRs that touch core and may need additional validation. label Nov 7, 2023
@Jarno458 Jarno458 force-pushed the more_readable_test_errors branch from 255157a to 2a86ba0 Compare November 14, 2023 22:08
@Jarno458 Jarno458 changed the title Tests: Added more specific error messages to some asserts Tests: Added more specific error messages to the duplicate id asserts Nov 14, 2023
@Jarno458
Copy link
Collaborator Author

Updated to remove the GameName messages

.symmetric_difference(set(world_type.item_name_to_id.keys())) or \
set(world_type.item_id_to_name.keys()) \
.symmetric_difference(set(world_type.item_name_to_id.values()))
self.assertFalse(overlap)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a big fan of this style of creating boolean values and asserting them, when the real check is actually the if. What was wrong with the suggested code above?

@agilbert1412 agilbert1412 added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Feb 12, 2024
@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Mar 28, 2024
@Jarno458
Copy link
Collaborator Author

Jarno458 commented Apr 8, 2024

Oke lets close this i guess

@Jarno458 Jarno458 closed this Apr 8, 2024
@Jarno458 Jarno458 deleted the more_readable_test_errors branch April 8, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: refactor/cleanup Improvements to code/output readability or organizization. waiting-on: author Issue/PR is waiting for feedback or changes from its author. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants