Split testWriteHtmlSafe into Two Separate Test Cases for Improved Test Granularity

[Codegass]

Problem solved by the feature

I would like to propose a minor improvement for the MixedStreamTest.testWriteHtmlSafe test case.

Currently, the MixedStreamTest.testWriteHtmlSafe unit test in Gson's suite checks both the behavior of Gson with HTML escaping enabled and disabled within a single test method.

  @Test
  public void testWriteHtmlSafe() {
    List<String> contents = Arrays.asList("<", ">", "&", "=", "'");
    Type type = new TypeToken<List<String>>() {}.getType();

    StringWriter writer = new StringWriter();
    new Gson().toJson(contents, type, new JsonWriter(writer));
    assertThat(writer.toString())
        .isEqualTo("[\"\\u003c\",\"\\u003e\",\"\\u0026\",\"\\u003d\",\"\\u0027\"]");

    writer = new StringWriter();
    new GsonBuilder().disableHtmlEscaping().create().toJson(contents, type, new JsonWriter(writer));
    assertThat(writer.toString()).isEqualTo("[\"<\",\">\",\"&\",\"=\",\"'\"]");
  }

This approach, while effective, combines two distinct scenarios into one test. When the first assertion fails, it prevents the subsequent assertions from running, which can obscure the presence of multiple issues under different scenarios. This can also make pinpointing the exact cause of failures slightly more challenging and may slightly increase the time taken to run the test due to combined scenarios

Feature description

I propose splitting the testWriteHtmlSafe test case into two separate tests: testWriteHtmlSafeWithEscaping for testing Gson's behavior with HTML escaping enabled and testWriteHtmlSafeWithoutEscaping with it disabled.

@Test
public void testWriteHtmlSafeWithEscaping() {
    List<String> contents = Arrays.asList("<", ">", "&", "=", "'");
    Type type = new TypeToken<List<String>>() {}.getType();

    StringWriter writer = new StringWriter();
    new Gson().toJson(contents, type, new JsonWriter(writer));
    assertThat(writer.toString()).isEqualTo("[\"\\u003c\",\"\\u003e\",\"\\u0026\",\"\\u003d\",\"\\u0027\"]");
}

@Test
public void testWriteHtmlSafeWithoutEscaping() {
    List<String> contents = Arrays.asList("<", ">", "&", "=", "'");
    Type type = new TypeToken<List<String>>() {}.getType();

    StringWriter writer = new StringWriter();
    new GsonBuilder().disableHtmlEscaping().create().toJson(contents, type, new JsonWriter(writer));
    assertThat(writer.toString()).isEqualTo("[\"<\",\">\",\"&\",\"=\",\"'\"]");
}

This change will enhance the granularity of our testing by isolating each scenario into its own test. Such isolation ensures that a failure in one scenario does not prevent the execution of the other, allowing for clearer identification of issues across different scenarios.

Additionally, having more granular tests could potentially reduce individual test running times, making the testing process more efficient, especially when debugging and running tests repeatedly during development.

Alternatives / workarounds

---

Hope this suggestion is helpful, and if yes, I am more than happy to submit a PR to implement the changes.

[eamonnmcmanus]

That does seem like an improvement. A PR would be welcome.

For these test improvements, I think it is OK just to send a PR without creating an issue first. We can discuss the merits of the change in the PR conversation.

[Codegass]

Thanks, I will directly go for the PR next time.