chore: refactor assertions to use assertThatThrownBy

[robfrank]

This pull request refactors test code across multiple modules to improve readability and consistency by replacing manual exception handling with AssertJ's assertThatThrownBy. The changes streamline how exceptions are asserted in tests, making them more idiomatic and easier to maintain. Additionally, some minor code cleanups and formatting improvements are included.

Test exception assertion improvements:

• Replaced manual try/catch blocks and calls to Assertions.fail() with assertThatThrownBy for exception assertions in various test files, including ScriptExecutionTest.java, SQLGrammarErrorsTest.java, DictionaryTest.java, DocumentValidationTest.java, and DropTypeTest.java. This change improves clarity and reduces boilerplate in test code. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Test code formatting and imports:

• Cleaned up imports in several test files by removing unused imports and organizing them, such as in ScriptExecutionTest.java, DictionaryTest.java, DocumentValidationTest.java, DropTypeTest.java, and GraphQLBasicTest.java. [1] [2] [3] [4] [5]
• Improved formatting of multi-line strings for GraphQL schema definitions in AbstractGraphQLTest.java and GraphQLBasicTest.java, replacing string concatenation with Java text blocks for better readability. [1] [2]

General code cleanup:

• Removed redundant code and simplified test setup, such as eliminating unnecessary methods and variables in test files. [1] [2]

These changes make the test codebase more maintainable and modern, leveraging best practices for exception assertions and code formatting.

[gemini-code-assist]

Summary of Changes

Hello @robfrank, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on modernizing and improving the test suite's quality and readability. The primary change involves adopting AssertJ's assertThatThrownBy for exception assertions, which streamlines test logic and reduces boilerplate. Additionally, the PR includes formatting improvements, such as utilizing Java text blocks for multi-line strings in GraphQL tests, and general code cleanup to enhance maintainability.

Highlights

• Refactored Exception Assertions: Replaced manual try/catch blocks and Assertions.fail() with AssertJ's assertThatThrownBy for more readable and concise exception testing across numerous test files.
• Improved Code Formatting: Converted multi-line string concatenations for GraphQL schema definitions into Java text blocks, significantly enhancing readability.
• Test Code Cleanup: Removed unused imports, simplified test setups, and eliminated redundant code, contributing to a cleaner and more maintainable test codebase.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request does a great job of refactoring the test suite to use AssertJ's assertThatThrownBy for exception testing, which significantly improves readability and maintainability. The switch to Java text blocks for multi-line strings is also a welcome improvement. I've found a few minor areas for improvement where the exception type in assertions could be more specific to make the tests even more robust. Overall, this is a solid refactoring effort.

[gemini-code-assist]

Using Exception.class is too broad and can make the test pass for unintended exceptions. The updateName method on the dictionary is expected to throw an IllegalArgumentException when used with a type name. It's better to assert for the specific exception type to make the test more robust.

Suggested change

	assertThatThrownBy(() -> database.transaction(() -> {
	assertThat(database.getSchema().existsType("V")).isTrue();
	database.getSchema().getDictionary().updateName("V", "V2");
	})).isInstanceOf(Exception.class);
	assertThatThrownBy(() -> database.transaction(() -> {
	assertThat(database.getSchema().existsType("V")).isTrue();
	database.getSchema().getDictionary().updateName("V", "V2");
	})).isInstanceOf(IllegalArgumentException.class);

[gemini-code-assist]

While this is a good refactoring, using Exception.class is too broad. The checkDocumentWasCreated method is declared to throw an IOException when an HTTP error occurs. Asserting for the specific IOException makes the test more precise and prevents it from passing on other unexpected runtime exceptions.

Suggested change

	assertThatThrownBy(() -> checkDocumentWasCreated(DATABASE_NAME, serverIndex, payload, rid, null))
	.isInstanceOf(Exception.class);
	assertThatThrownBy(() -> checkDocumentWasCreated(DATABASE_NAME, serverIndex, payload, rid, null))
	.isInstanceOf(IOException.class);

[gemini-code-assist]

Asserting for a generic Exception is not ideal as it can mask other issues. Since an HTTP error is expected, which results in an IOException from readResponse, it would be better to assert for IOException.class specifically. This makes the test more robust.

Suggested change

	assertThatThrownBy(() -> readResponse(connection))
	.isInstanceOf(Exception.class)
	.hasMessageContaining("400");
	assertThatThrownBy(() -> readResponse(connection))
	.isInstanceOf(IOException.class)
	.hasMessageContaining("400");