Add test coverage

PART_1_SHARE_INFO.md

@@ -195,9 +195,10 @@ install them one at a time if you want to know more about what we're doing here.
 ### Have a look at the project
 
 In the file explorer on the left of the VS Code window,
-open up `src -> main/java/hellos -> Hellos.java` and you should see a small
-Java program. Now open up `src -> test/java/hellos -> HellosTest.java` and you
-should see our JUnit tests.
+open up `src -> main/java/hellos -> Main.java` and
+ `src -> main/java/hellos -> Hellos.java`, and you should see a small
+Java program. Now open up `src -> test/java/hellos -> HellosTest.java`
+and you should see our JUnit tests.
 
 > [JUnit is a widely used testing framework for Java](https://junit.org/junit5/docs/current/user-guide/#writing-tests).
 > A Junit test is a method that has the special `@Test` annotation and contains

PART_2_JAVA_INTRODUCTIONS.md

@@ -329,11 +329,11 @@ branch dropdown.
 If you select it you should see your last commit just below
 that. Next to the "Latest commit" label should either be an
 orange/yellow dot
-![GitHub's orange in-progress circle][orange-circle],
+(![GitHub's orange in-progress circle][orange-circle]),
 a green check mark
-![GitHub's green success check mark][green-check],
+(![GitHub's green success check mark][green-check]),
 or a red x
-![GitHub's red failure x][red-x]
+(![GitHub's red failure x][red-x])
 These show you the build status of that commit.
 Every time you (or anyone else) pushes to GitHub, we have GitHub Actions
 set up so they run the JUnit tests against that version of the code.
@@ -347,7 +347,7 @@ The meaning of these symbols is:
   the tests passed, and any other checks passed as well).
 - ![GitHub's red failure x][red-x]: A red x means that
   something bad happened, like a test failed.
-  
+
 If you click on any of these symbols on the GitHub page, you can get
 more information, including links to pages with details on, e.g., failed tests.
 
@@ -426,10 +426,13 @@ and project repositories in this course. All those checks have to pass before
 you can merge in your pull request. For this lab we have four checks:
 
 - "build": This runs `./gradlew check`, which compiles the code
-  and runs two checks. If both of these succeed you get a green check mark, and you get a red x if anything fails.
+  and runs several checks. If all of these succeed you get a check mark (:white_check_mark:), and you get a red x (:x:) if anything fails.
   - It uses a tool called [Checkstyle](https://checkstyle.org/) that
     checks that your code follows a set of basic style guides.
   - It runs the JUnit tests
+  - It confirms that the test coverage is at least 80%. (This should
+    be trivially true as the tests that we've provided should
+    automatically cover any new code that's added to `Hellos.java`)
 - "markdown-link-check": This checks that all the links in the
   project's Markdown files "work" (i.e., there's something at
   the other end of that URI). You won't be adding or changing any
@@ -442,10 +445,10 @@ So have a look at the bottom of your pull request and you should see the status
 then that tells you that check is still being processed (e.g., the tests are still
 being run) so you'll need to wait a bit for the check to finish. The checks can
 take a few minutes, depending on their complexity and how busy the various services
-are. That's one of several reasons you should usually run the tests locally before
+are. :bangbang: That's one of several reasons you should usually run the tests locally before
 you push.
 
-If any of the checks fail (give you a red x), then you probably want to click on
+If any of the checks fail (give you a red :x:), then you probably want to click on
 "Details" by that check to learn more about what might have failed.
 
 - If the Java `build` fails, it's likely because a test has
@@ -514,21 +517,21 @@ There are numerous ways this conflict could have come about, such as:
   instead call the (supposedly different) method `chrisSaysHello()`.
 - Chris has renamed `patSaysHello()` to `chrisSaysHello()`, and wants to change
   the call to match this renaming.
-- Chris wants to make _both_ function calls (in some order, which `git` couldn't
-  begin to guess).
+- Chris wants to make _both_ function calls in some order, and `git` can't
+  begin to guess what the desired is.
 
 Here `git` can't tell which of these happened, and thus can't "know" how to
-handle the merge. Thus it punts it to us, the human users, to sort out. Sometimes
+handle the merge. Thus it punts it to us, the human developers, to sort out. Sometimes
 it's obvious to us what to do, but sometimes it's really _not_ clear what the
 right course of action is. :warning: DO NOT JUST THROW AWAY OTHER PEOPLE'S
-WORK TO RESOLVE A MERGE CONFLICT. This is a place where you really want to
+WORK TO RESOLVE A MERGE CONFLICT :warning:. This is a place where you really want to
 take a moment and contact other people on your team to decide what is the best course
 of action.
 
 In this lab, the last option is the most likely situation, and we better call `chrisSaysHello()`
 first so that at least those two calls are in alphabetical order.
 
-Chris would use the web editor to make those changes, deleting the `<<<<`, `====`,
+Chris could use the web editor to make those changes, deleting the `<<<<`, `====`,
 and `>>>>` lines, leaving the two calls in the desired order. When things look good,
 Chris clicks "Mark as resolved", and then the button to commit the merged changes.
 
@@ -541,17 +544,20 @@ for more examples and details.
 
 :warning: :bangbang: At this point Chris (or you if you've been doing something
 similar) will need to pull these changes back down to your copy if you want
-or need to make changes to your branch. If, for example, Chris realizes here
+or need to make changes to your branch. The changes you've made by
+resolving conflicts in the web GUI _only_ affect the version of the
+code on GitHub, so you have to pull if you want those changes to be
+reflected in your local copy as well. If, for example, Chris realizes here
 that the tests are failing because they have the alphabetical order wrong,
 they'll need to pull down these changes, fix the alphabetical order, push back
 up, and attempt to merge again.
 
 #### You need a review
 
 Now you've finally gotten your code up-to-date and all the checks pass. You
-still can't merge because of a big red X and the message "Review required".
+still can't merge because of a red :x: and the message "Review required".
 This is because we're following a common "best practice" and requiring at
-least two code review _by other people_ for each pull request.
+least two code reviews _by other people_ for each pull request.
 
 So you need to get someone to review your code! In the upper right there's
 a "Reviewers" section. Click on the gear there and you should get a drop down
@@ -595,7 +601,7 @@ check for:
 - [ ] Do the new methods they're adding have reasonable names that convey useful
   info to the reader?
 - [ ] Are new method names in camel cases, starting in lowercase (the Java standard
-  for method names)?.
+  for method names)?
 - [ ] Is the implementation of the new methods reasonable?
 - [ ] Is the spacing and indentation uniform and consistent with the rest of the file?
 
@@ -608,13 +614,23 @@ gets merged in. _Don't be shy about requesting changes._ It can seem kind of mea
 and it definitely slows things down, but it can significantly improve the quality
 of the overall work, which benefits everyone in the long run.
 
+> :warning: Don't just gloss over their code. It is ultimately up to
+_all_ of us to work together to maintain the overall quality of the
+work produced by our groups. Part of that is taking code reviews
+seriously. It may not seem "nice" to ask for changes to someone's
+pull request, but this is the best (and often the only) time for
+us to help each other improve the quality of our work. Suggesting
+that they fix a misspelling or improve the naming of a method may
+seem "minor" and kinda obnoxious, but really that's how we maintain
+the quality of our systems over time. So be polite, but also be firm.
+
 #### Process the code reviews you receive
 
 Once you receive a positive review you can proceed to merge. If, however, you
 received a request to make some changes, look those over. Does the request make
 sense? Do you understand what (and why) it's being made? If not, _definitely_
 ask the requestor for further information or explanation. You can post your own comment
-on the pull request and/or contact them by other means (e.g., Slack, email). You
+on the pull request and/or contact them by other means (e.g., talk to them in person, or use things like Discord or email). You
 probably want any important info to be captured in the pull request, but sometimes
 it's useful to poke someone on another channel to get their attention.
 
@@ -634,7 +650,7 @@ be visible in `main` – your work is officially part of the project! You'll
 be given the option to "Delete branch", which will delete your work branch.
 Since the changes on that branch are now all merged into `main` it should be
 safe to delete that branch, but you can leave it there if you prefer and
-remove it later in a "branch housekeeping" session.
+remove it in some "branch housekeeping" session you run later.
 
 ## Huzzah! We're done! :-)
 

build.gradle

@@ -9,6 +9,9 @@ plugins {
   // Apply the application plugin to add support for building a CLI application.
   id 'application'
 
+  // Apply the Jacoco plugin to add suppport for JUnit test coverage reports.
+  id 'jacoco'
+
   // The checkstyle plugin provides basic Java style checks to ensure we're
   // following industry style standards.
   id 'checkstyle'
@@ -38,12 +41,86 @@ dependencies {
 
 application {
   // Define the main class for the application.
-  mainClass.set('hellos.Hellos')
+  mainClass.set('hellos.Main')
 }
 
 test {
   // Use junit platform for unit tests.
   useJUnitPlatform()
+
+  // After running the tests, generate a coverage report
+  finalizedBy jacocoTestReport
+  // After running the tests, check the coverage level
+  finalizedBy jacocoTestCoverageVerification
+}
+
+jacocoTestReport {
+  // Running the test report task automatically runs test first
+  dependsOn test
+
+  reports {
+    // This isn't strictly necessary, but the default reports
+    // location is buried pretty deep in the build directory,
+    // so this makes it easier to find.
+    html.outputLocation = file("${buildDir}/jacocoHtml")
+  }
+
+  afterEvaluate {
+    // This excludes the `Main` class from coverage verification.
+    // All the "useful" logic is in the `Hellos` class, so we'll
+    // check the coverage on that class instead.
+    classDirectories.setFrom(files(classDirectories.files.collect {
+      fileTree(dir: it, exclude: ['**/Main.class'])
+    }))
+  }
+}
+
+jacocoTestCoverageVerification {
+  // Running the test verification task automatically runs test first
+  dependsOn test
+
+  // These are the rules applied to the test coverage
+  violationRules {
+    rule {
+      // We are looking at the entire bundle overall, i.e.,
+      // 80% of all instructions across all the Java files
+      // need to be covered.
+      element = 'BUNDLE'
+
+      // 80% of instructions should be covered
+      limit {
+        counter = 'INSTRUCTION'
+        minimum = 0.8
+      }
+
+      // 80% of lines should be covered
+      limit {
+        counter = 'LINE'
+        minimum = 0.8
+      }
+
+      // 80% of branches should be covered
+      limit {
+        counter = 'BRANCH'
+        minimum = 0.8
+      }
+
+      // 80% of methods should be covered
+      limit {
+        counter = 'METHOD'
+        minimum = 0.8
+      }
+    }
+  }
+
+  // This excludes the `Main` class from coverage verification.
+  // All the "useful" logic is in the `Hellos` class, so we'll
+  // check the coverage on that class instead.
+  afterEvaluate {
+    classDirectories.setFrom(files(classDirectories.files.collect {
+      fileTree(dir: it, exclude: ['**/Main.class'])
+    }))
+  }
 }
 
 tasks.withType(JavaCompile) {

src/main/java/hellos/Hellos.java

@@ -6,16 +6,6 @@
  */
 public class Hellos {
 
-  public static void main(final String[] args) {
-    Hellos helloRunner = new Hellos();
-    helloRunner.run();
-  }
-
-  public void run() {
-    String output = generateOutput();
-    System.out.println(output);
-  }
-
   public String generateOutput() {
     StringBuilder builder = new StringBuilder();
 

src/main/java/hellos/Main.java

@@ -0,0 +1,11 @@
+package hellos;
+
+public class Main {
+
+    public static void main(final String[] args) {
+      Hellos helloRunner = new Hellos();
+      String output = helloRunner.generateOutput();
+      System.out.println(output);
+    }
+
+}

src/test/java/hellos/HellosTest.java

@@ -26,10 +26,10 @@ public void testLineStructure() {
     // Taken from
     // https://stackoverflow.com/questions/15805555/java-regex-to-validate-full-name-allow-only-spaces-and-letters
     // The pattern needs to include
-    // * some name,
-    // * the "says",
-    // * and the single quotes, and
-    // * they must speak with emphasis as noted by the required "!"
+    //  - some name,
+    //  - the "says",
+    //  - and the single quotes, and
+    //  - they must speak with emphasis as noted by the required "!"
     String linePattern = "[\\p{L} .'-]+ says '+[\\p{L} .'-]+!'";
 
     for (int i = 0; i < lines.length; ++i) {