-
Notifications
You must be signed in to change notification settings - Fork 3k
API, Build: Explicitly pass version into Git Properties Plugin #12949
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,11 @@ | |
| package org.apache.iceberg; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assumptions.assumeThat; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import java.util.Locale; | ||
| import java.util.regex.Pattern; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
@@ -37,6 +41,34 @@ public void testFullVersion() { | |
| + ")"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testVersionNotUnspecified() { | ||
| assertThat(IcebergBuild.version()).isNotEqualTo("unspecified"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testVersionMatchesSystemProperty() { | ||
| assumeThat(System.getProperty("project.version")).isNotNull(); | ||
| assertThat(IcebergBuild.version()) | ||
| .isEqualTo(System.getProperty("project.version")) | ||
| .as("IcebergBuild.version() should match system property project.version"); | ||
| } | ||
|
|
||
| /** | ||
| * This test is for Source Releases. When we have a source release we use a version.txt file in | ||
| * the parent directory of this module to actually set the "version" which should be included in | ||
| * the gradle build properties used by IcebergBuild. | ||
| */ | ||
| @Test | ||
| public void testVersionMatchesFile() throws IOException { | ||
| Path versionPath = Paths.get("../version.txt").toAbsolutePath(); | ||
| assumeThat(java.nio.file.Files.exists(versionPath)).isTrue(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: If we set the we can always check This way we don't need two testcase and also it won't be a conditional test.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's a good suggestion, but I really want to test that file. I'm trying to test against the thing we know must be correct. We basically directly pipe this value into version.txt in our source-release script. If we somehow bungle gradle in the future and that value doesn't match the text file for whatever reason I'd like to know. I can add this test as well as an assert though |
||
| String versionText = java.nio.file.Files.readString(versionPath).trim(); | ||
| assertThat(IcebergBuild.version()) | ||
| .isEqualTo(versionText) | ||
| .as("IcebergBuild.version() should match version file"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testVersion() { | ||
| assertThat(IcebergBuild.version()).as("Should not use unknown version").isNotEqualTo("unknown"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajantha-bhat Added this test, all good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks