Pytest validation #3123
Pytest validation #3123
Conversation
…GPIO" are now updated to "Gpio".
Fixes that need to be made in future commits: - build errors across several validation testing files - unnecessary comments need to be erased
Bugs: - several validation files don't build
…o not enter a region. Incomplete process but committing prior to laptop dying.
…est-validation. Also ran formatting script. # Conflicts: # src/software/simulated_tests/BUILD # src/software/simulated_tests/ball_enters_region.py # src/software/simulated_tests/or_validation.py # src/software/simulated_tests/robot_enters_region.py
…pe. Ex: ALWAYS validation and EVENTUALLY validation can both be passed to or_validation.
…pe. Ex: ALWAYS validation and EVENTUALLY validation can both be passed to or_validation.
| if validation_type != validation_type_initial: | ||
| raise TypeError("type of validation instances is not consistent") | ||
| return validation_type_initial | ||
| return ValidationType |
There was a problem hiding this comment.
| return ValidationType | |
| return self.validations[0].get_validation_type(world) |
| if validation_type != validation_type_initial: | ||
| raise TypeError("type of validation instances is not consistent") | ||
| return validation_type_initial | ||
| return ValidationType |
There was a problem hiding this comment.
I think the constructor should check that the validation types are consistent
def __init__(self, validations):
"""An OR extension to the validation function"""
assert len(validations) > 0
validation_type_initial = validations[0].get_validation_type()
for validation in validations:
validation_type = validation.get_validation_type()
if validation_type != validation_type_initial:
raise TypeError("Type of validation instances is not consistent")
self.validations = validations
There was a problem hiding this comment.
Why do validation types put into the function need to be consistent?
If we wanted to check if either an always validation or eventually validation are passing, then the OrValidation class would not be able to support this (provided that this check is kept).
There was a problem hiding this comment.
Our underlying validation code doesn't support mixing eventually and always validations:
. So, if we try to do something like that, we should warn the programmer that it probably won't work
|
|
||
|
|
||
| @pytest.mark.parametrize("is_friendly_test", [True, False]) | ||
| # TODO 3119 Fix KickoffEnemyPlay |
There was a problem hiding this comment.
this TODO has been fixed, you can delete it
…ots are in friendlyHalf + centerCircle is now within the if is_friendly_test and else blocks. Added BallAlwaysMoveFromRest() validation to OrValidation() with NumberOfRobotsAlwaysStaysInRegion() so that once the ball moves, the test still passes if the robots enters regions that would have previously made the test fail. Added debugging statements to ball_moves_from_rest and or_validation. Pending removal.
|
Note: pushed PR right now to allow others to debug. Not yet ready for review to merge PR. |
Remaining issues: - enemy robot performs kickoff during friendly kickoff - friendly robots enter enemy region when test is running with -t
|
This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
|
This PR was closed because it has been stalled for 5 days with no activity. |
| req_robot_cnt=6, | ||
| ), | ||
| ] | ||
| ) | ||
| ) |
There was a problem hiding this comment.
you could reduce the duplication in this code a bit:
ball_moves_at_rest_validation = BallAlwaysMovesFromRest(
position=tbots_cpp.Point(0, 0),
threshold=0.01
)
expected_centre_circle_or_validations_set = [
ball_moves_at_rest_validation,
NumberOfRobotAlwaysStaysInRegion(
regions = [
tbots_cpp.Field.createSSLDivisionBField().centerCircle()
],
req_robot_cnt = 0
)
]
friendly_regions = [tbots_cpp.Field.createSSLDivisionBField().friendlyHalf()]
if friendly_test:
expected_centre_circle_or_validations_set.append(
NumberOfRobotsAlwaysStaysInRegion(
regions=[
tbots_cpp.Field.createSSLDivisionBField().centerCircle()
],
req_robot_cnt = 1
)
)
else:
friendly_regions.append(tbots_cpp.Field.createSSLDivisionBField().centerCircle())
expected_robot_regions_or_validations_set = [
ball_moves_at_rest_validation,
NumberOfRobotAlwaysStaysInRegion(
regions=friendly_regions,
req_robot_cnt=6
)
]
always_validation_sequence_set[0] = [
OrValidation(expected_centre_circle_or_validations_set)),
OrValidation(expected_robot_regions_or_validations_set)
]
| OrValidation( | ||
| [ | ||
| BallAlwaysMovesFromRest( | ||
| position=tbots_cpp.Point(0, 0), threshold=0.01 |
There was a problem hiding this comment.
the SSL robocup threshold is 0.05 m
https://robocup-ssl.github.io/ssl-rules/sslrules.html#_double_touch
| OrValidation( | ||
| [ | ||
| BallAlwaysMovesFromRest( | ||
| position=tbots_cpp.Point(0, 0), threshold=0.01 |
There was a problem hiding this comment.
The SSL Robocup threshold is 0.05 m: https://robocup-ssl.github.io/ssl-rules/sslrules.html#_double_touch
| OrValidation( | ||
| [ | ||
| BallAlwaysMovesFromRest( | ||
| position=tbots_cpp.Point(0, 0), threshold=0.01 |
There was a problem hiding this comment.
SSL Robocup threshold is 0.05 m
https://robocup-ssl.github.io/ssl-rules/sslrules.html#_double_touch
Please fill out the following before requesting review on this PR
Description
This PR addresses issue #2809. kickoff_play_test.py has been written to test the following rules for kickoff:
kickoff_play_test.py uses NumberOfRobotsAlwaysStaysInRegion() in the always validation sequence set and BallEventuallyExitsRegion() in the eventually validation sequence set to verify the above rules.
robot_enters_regions.py was modified so that NumberOfRobotsAlwaysStaysInRegion() is able to accept multiple regions as parameters.
or_validation.py was created to implement OR logic when running a group of tests - this is used in kickoff_play_test.py to check that either 0 or 1 robots are in centerCircle.
This test discovered that our AI currently violates kickoff rule #3 outlined above - a friendly robot enters the enemy half during enemyKickoff. As such, issue #3119 was created.
Testing Done
./tbots.py test //software/ai/hl/stp/play:kickoff_play_test -t was used to visualize the kickoff_play_test on Thunderscope. All three tests can be seen in this GIF:
Resolved Issues
resolves #2809
Length Justification and Key Files to Review
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issue