Add --suite flag to tbots.py to run entire test suites

[nycrat]

Description

This PR adds a --suite / -s flag to tbots.py which runs all the software and simulated gameplay tests like ci does.

Testing Done

Ran all the tests using ./tbots.py test --suite and ./tbots.py test -s. Old behaviour when only running or building a single target still works.

Resolved Issues

resolves #3585

Length Justification and Key Files to Review

N/A

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

• Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
• Remove all commented out code
• Remove extra print statements: for example, those just used for testing
• Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

[StarrryNight]

Looks good to me. Will we want to add other test suites like Robot Software or AutoRef'd game?

[williamckha]

LGTM just one question

[williamckha]

is the -- necessary? It's not present in the simulated_gameplay_tests

[nycrat]

It's required since the command it uses the - to ignore some bazel targets and it doesn't work without having -- first

[williamckha]

Looks good to me. Will we want to add other test suites like Robot Software or AutoRef'd game?

Probably not necessary, robot software tests in CI don't do much other than build thunderloop and run redis tests. We also already have an autoref option you can add when running thunderscope.

[adrianchan787]

looks good i think!

[nycrat]

@Andrewyx what do you think? I know you suggested potentially not using a flag at all and just adding some specific targets for test suites which would be documented/autocompleted by tbots.py. I think it would be beneficial to make it easier to find the list of possible test suites that it can run, but I'm more inclined to do it another way, maybe refactoring tbots.py so it can actually have multiple commands and creating a suite command and it will show the targets in the help/autocomplete for that specific command

[Andrewyx]

@Andrewyx what do you think? I know you suggested potentially not using a flag at all and just adding some specific targets for test suites which would be documented/autocompleted by tbots.py. I think it would be beneficial to make it easier to find the list of possible test suites that it can run, but I'm more inclined to do it another way, maybe refactoring tbots.py so it can actually have multiple commands and creating a suite command and it will show the targets in the help/autocomplete for that specific command

Yeah upon reviewing this code again, I don't see a reason we should be using fuzzy finder for this given we only have 2 test suites, and it is unlikely we are going to have more in the future (at most 1 more if we design onboard test suites).

Digging into the semantics of this design. Considering the purpose of fuzzy finder, which is to reduce the cognitive overhead of explicitly remembering particular tests, its use case is not really beneficial when only 2 options are in our search space. This means that developers will be responsible for remembering these 2 options (even if not 1:1 exactly thanks to fuzzy find). Ideally, we should design this invocation to be as intuitive as possible, and reduce the load being placed on users to remember specific targets. This is my reasoning for how this task might best be approached.

Here are an option I thought of for this UX if you want my brainstorming:

• ./tbots.py test by default runs the test suites or ./tbots.py test . if that makes more sense?

[nycrat]

Here are an option I thought of for this UX if you want my brainstorming:

• ./tbots.py test by default runs the test suites or ./tbots.py test . if that makes more sense?

Yea I think ./tbots.py test running a single comprehensive test suite would make sense, because the typical use case would be that you want to make sure your changes pass all ci tests without having to wait an hour. It is not often that you would need to run only the software unit tests or the simulated gameplay tests

[nycrat]

Actually I think ./tbots.py test --suite running all the test suites would be better since it is more explicit and it will show in the --help

[nycrat]

Oh wait now that I'm thinking about it, we could just run bazel test //... to run all the tests... not sure if this feature is still necessary?

[GrayHoang]

I like it, simply because it means we can unify one interface for all testing, building, and running. Although I think our fuzzy finder still needs some more work, this is a good step so that people don't have to remember so many bazel commands.