Skip to content

Improve tests #102

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.

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

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Improve tests #102

merged 4 commits into from
Feb 23, 2024

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