Skip to content

Conversation

Marcono1234
Copy link
Contributor

Context

Tries to improve the unit tests; in particular:

  • Fix PropertiesFileGeneratorTest failing on Windows
  • Fix GitCommitIdPluginIntegrationTest not using assertThat(...) correctly & simplify assertions

See the commit messages for details. Feedback is appreciated!

Contributor Checklist

  • Added relevant integration or unit tests to verify the changes
  • Update the Readme or any other documentation (including relevant Javadoc)
  • Ensured that tests pass locally: mvn clean package
  • Ensured that the code meets the current checkstyle coding style definition: mvn clean verify -Pcheckstyle -Dmaven.test.skip=true -B

`--release` also verifies that only API from that Java version is used.
Otherwise when building with a JDK newer than the target version it would
be possible to accidentally use API not available in the target version.
The properties files are written with OS-specific line breaks, but the
expected output previously only considered '\n' instead  of Windows '\r\n'.

Also simplified the code reading the properties file content in
PropertiesFileGeneratorTest.
This AssertJ method requires that afterwards you write the actual assertion,
e.g. `assertThat(value).isFalse()`

`assertThat(value)` on its own does not do anything and always passes.
… id mode

`CommitIdGenerationMode.FLAT` is the default but some of the tests seem to
erroneously expect the full format by default.

This was previously not detected because that test class was using
`assertThat` incorrectly.
@TheSnoozer
Copy link
Contributor

Thanks for your contributions! I never ran the tests on windows so thanks for fixing that :-)

Changes look good for me, so thanks again I'm going ahead and merge!

@TheSnoozer TheSnoozer merged commit b4dd65a into git-commit-id:master Feb 23, 2024
@TheSnoozer TheSnoozer added this to the 6.0.0 milestone Feb 23, 2024
@Marcono1234 Marcono1234 deleted the improve-tests branch February 24, 2024 16:16
@TheSnoozer TheSnoozer modified the milestones: 6.0.0, 6.0.0-rc.7 Mar 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants