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

Add Unit Testing article #1203

Merged
merged 23 commits into from
Feb 27, 2021
Merged

Add Unit Testing article #1203

merged 23 commits into from
Feb 27, 2021

Conversation

Starlight220
Copy link
Member

Split from #1077

I'll add some cleanup input I got from @ThadHouse, and I'd like help with the C++ parts.

Copy link
Member

@Daltz333 Daltz333 left a comment

Choose a reason for hiding this comment

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

Additionally, can you copy and paste this class and verify that all the code runs as expected? I found a if/else syntax error and could miss other things.

@jasondaming
Copy link
Member

I think this sentence sounds awkward: "There are many unit testing frameworks for most languages". "Unit testing is very common and most languages have multiple frameworks" or similar would be my suggestion.

I think it is important to mention under "Writing Testable Code" that they can't define their hardware inline if they want to use unit tests. E.g. private final WPI_TalonSRX m_talonsrxleft = new WPI_TalonSRX(DriveConstants.kLeftMotor2Port);

I think it would be also good to mention / show how a project needs to use methods to break apart their code so that individual sections can be tested. I am just trying to keep the "I am doing everything in "main" user in mind."

What do you think about mentioning Mockito? I (and I know others) have been using it (admittedly not as much as I would like) and I think it makes things significantly easier.

@Starlight220
Copy link
Member Author

I agree about the splitting into methods part, though I'm not sure how critical it is an FRC context as each subsystem action is pretty obviously a separate method.

What do you mean about inline hardware definitions?

I'm not sure how Mockito comes into play here, but I've never used it.

@jasondaming
Copy link
Member

So I guess what I am saying is that your code looks great but for teams that aren't set up with subsystems (lowercase) providing and example might help. I will make a quick example.

Many teams do:

if (m_controller.getYButtonPressed()) {
   if (spark.get() >= .75) {
      spark.set(0.75);
   }
   else {
      spark.set(0);
   }
}

To use unit testing it will need to be:

if (m_controller.getYButtonPressed()) {
   toggleIntakeMotor();
}

public void toggleIntakeMotor() {
   if (spark.get() >= .75) {
      spark.set(0);
   }
   else {
      spark.set(0.75);
   }
}

Mockito allows you to mock() your hardware and not have to worry about the closing stuff. Amongst a lot of other things. Here is a good video on using Mockito in FRC that we could include.

@Starlight220
Copy link
Member Author

Starlight220 commented Feb 18, 2021

Template robot projects don't have Mockito, that's already a big reason against documenting its usage in an FRC context. If it doesn't have a C++ equivalent, then it definitely won't be documented/suggested.
Specifically about that video - it's terribly outdated and from before all the sim stuff was added. Part of the reason that all the XxxSim classes exist is to make mocking hardware unnecessary and provide a better, more appropriate alternative.

Regarding your example, there are classes to simulate joystick input (see GenericHIDSim and the like). The current example shows the most basic case, given that it's more a tutorial than an example. Adding an integration test example/tutorial (perhaps similar to https://github.com/wpilibsuite/allwpilib/blob/main/wpilibj/src/test/java/edu/wpi/first/wpilibj/TimedRobotTest.java) that tests buttons or whatever can also be done but should be a later separate PR.

@Starlight220
Copy link
Member Author

Starlight220 commented Feb 19, 2021

given wpilibsuite/GradleRIO#497, it would seem that Gradle never runs tests on deploy. The VS Code option might though.

I don't have access to a robot, can someone check this?

@jasondaming
Copy link
Member

I think there should be a warning about the lack of support from 3rd party vendors about .close and any other things lacking implementation.

I had a lot of trouble with import static org.junit.Assert.*; "The import org.junit.Assert cannot be resolved"

@Starlight220
Copy link
Member Author

@Daltz333 is there anything else you'd like me to add, or can this be reviewed/merged?

@Daltz333 Daltz333 merged commit 46c217f into wpilibsuite:master Feb 27, 2021
@Starlight220 Starlight220 deleted the unit-test branch February 27, 2021 20:55
TheTripleV pushed a commit to TheTripleV/frc-docs that referenced this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants