Skip to content

added robot init unit test #6

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

Open
Lyssia-Seiden wants to merge 4 commits into
base: main
Choose a base branch
from
Open

added robot init unit test #6

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aa4bc95
added robot init unit test
LewisSeiden Jan 13, 2024
1b8e46f
run spotless
Jan 13, 2024
8ec3ef2
rename init check and make properly check for init errors
LewisSeiden Jan 13, 2024
cd7621c
Merge branch 'unit-tests' of https://github.com/HighlanderRobotics/Cr…
LewisSeiden Jan 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@
},
],
"java.test.defaultConfig": "WPIlibUnitTests",
"java.compile.nullAnalysis.mode": "automatic"
"java.compile.nullAnalysis.mode": "automatic",
"wpilib.skipTests": true

@kevinclark kevinclark Jan 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this set to true on purpose?

@Lyssia-Seiden Lyssia-Seiden Jan 13, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, to avoid excessive delays during deploys from running unit tests, and instead my plan was to run these tests in ci

}
2 changes: 1 addition & 1 deletion .wpilib/wpilib_preferences.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"enableCppIntellisense": false,
"currentLanguage": "java",
"projectYear": "2024beta",
"projectYear": "2024",
"teamNumber": 8033
}
25 changes: 25 additions & 0 deletions src/test/java/frc/InitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) FIRST and other WPILib contributors.
// Open Source Software; you can modify and/or share it under the terms of
// the WPILib BSD license file in the root directory of this project.

package frc;

import edu.wpi.first.hal.HAL;
import frc.robot.Robot;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

/** Test robot init. */
public class InitTests {
@BeforeEach
private void setup() {
HAL.initialize(500, 0);
}

@Test
private void initTest() {

@kevinclark kevinclark Jan 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Prefer test names that say what is expected. These are never called, so you should be excessively verbose since it's what's going to communicate intent when it fails. F.e, "robotShouldNotCombustWhenFedFrechfries()" would be a totally reasonable unit test method name.

@kevinclark kevinclark Jan 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also note you don't event need test in the name - you've already communicated it's a test with the annotation

@kevinclark kevinclark Jan 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All that being said, what's your goal with this method? When does it fail? What does it tell you when it does?

@Lyssia-Seiden Lyssia-Seiden Jan 13, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the goal here is just to make sure Robot initializes without throwing an error, mainly as a simple template for unit tests as we (hopefully) add more throughout the season

@kevinclark kevinclark Jan 13, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Then I'd make it explicit with a assertDoesNotThrow. Communicates your intent and makes clear you didn't forget the assert along the way.

var robot = new Robot();
robot.startCompetition();
robot.close();
}
}