-
Notifications
You must be signed in to change notification settings - Fork 88
SBCOSS-489: Sunbird-Knowlg-asset-enrichment - add the GitHub actions to run test cases and code quality checks when a PR raised #817
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
Conversation
… failing for hardcoded blob URL's
… failing for hardcoded blob URL's
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
… failing for hardcoded blob URL's
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
…ity checks for asset-enrichment flink job
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.
Pull Request Overview
This PR introduces a GitHub Actions workflow to enforce builds, tests, coverage reports, and SonarQube analysis on every pull request, and refactors test configurations across modules to pull schema and blob paths from environment-driven config.
- Add
.github/workflows/pr-actions.yml
to build modules, run tests with JaCoCo coverage, and run SonarCloud analysis on PRs. - Refactor test suites (HTTPUtilSpec, DefinitionCacheTestSpec, ThumbnailSpec, AssetEnrichmentTaskTestSpec) to load
base-test.conf
and replace hard-coded URLs withSCHEMA_BASE_PATH
,BLOB_IMAGE_CONTENT_PATH
, andBLOB_VIDEO_CONTENT_PATH
. - Update test resource configs (
test.conf
,base-test.conf
) to declare schema and blob input keys as optional env vars.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
search-indexer/src/test/resources/test.conf | Switch schema block to use ${?SCHEMA_BASE_PATH} |
jobs-core/src/test/scala/org/sunbird/spec/HTTPUtilSpec.scala | Load base-test.conf and use blob.input.contentImagePath |
jobs-core/src/test/scala/org/sunbird/spec/DefinitionCacheTestSpec.scala | Switch hard-coded basePath to config-driven schema.basePath |
jobs-core/src/test/resources/base-test.conf | Add schema.basePath and blob.input.contentImagePath entries |
asset-enrichment/src/test/scala/org/sunbird/job/spec/ThumbnailSpec.scala | Replace URL literals with blob paths from config |
asset-enrichment/src/test/scala/org/sunbird/job/spec/AssetEnrichmentTaskTestSpec.scala | Refactor tests to use blob paths and dynamic metadata |
asset-enrichment/src/test/resources/test.conf | Declare schema.base_path , blob.input.* , and cloud storage keys |
.github/workflows/pr-actions.yml | New CI job: build, test, coverage, and SonarCloud analysis on PRs |
Comments suppressed due to low confidence (2)
asset-enrichment/src/test/scala/org/sunbird/job/spec/ThumbnailSpec.scala:11
- Class name typo:
ThubnailUtilSpec
should beThumbnailUtilSpec
to match the file name and intent.
class ThubnailUtilSpec extends BaseTestSpec {
asset-enrichment/src/test/resources/test.conf:38
- The key
base_path
is inconsistent with other modules (schema.basePath
). Rename tobasePath
to match the convention used elsewhere.
base_path = ${?SCHEMA_BASE_PATH}
@@ -9,9 +10,11 @@ import java.io.File | |||
class HTTPUtilSpec extends FlatSpec with Matchers { | |||
|
|||
val httpUtil = new HttpUtil | |||
val config: Config = ConfigFactory.load("base-test.conf") |
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.
[nitpick] Consider adding .withFallback(ConfigFactory.systemEnvironment())
so tests pick up environment variables like BLOB_IMAGE_CONTENT_PATH
when not defined in the file.
val config: Config = ConfigFactory.load("base-test.conf") | |
val config: Config = ConfigFactory.load("base-test.conf").withFallback(ConfigFactory.systemEnvironment()) |
Copilot uses AI. Check for mistakes.
|
||
"ThumbnailUtil.generateOutFile" should " return null for no file" in { | ||
val file = new ThumbnailUtilTest().generateOutFile(null, 150) | ||
file should be(None) | ||
} | ||
|
||
"ThumbnailUtil.generateOutFile" should " return None for the file" in { | ||
val imageUrl = "https://sunbirddev.blob.core.windows.net/sunbird-content-dev/content/do_113233717480390656195/artifact/bitcoin-4_1545114579639.jpg" | ||
val imageUrl = s"$imagePath" | ||
try { | ||
val file = FileUtils.copyURLToFile("do_113233717480390656195", imageUrl, imageUrl.substring(imageUrl.lastIndexOf("/") + 1)) |
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.
The first argument to copyURLToFile
should be the source URL (or new URL(imageUrl)
), not the contentId string. Consider using new URL(imageUrl)
and a File
for the destination.
val file = FileUtils.copyURLToFile("do_113233717480390656195", imageUrl, imageUrl.substring(imageUrl.lastIndexOf("/") + 1)) | |
val file = FileUtils.copyURLToFile(new URL(imageUrl), imageUrl, imageUrl.substring(imageUrl.lastIndexOf("/") + 1)) |
Copilot uses AI. Check for mistakes.
val newFile = new File(s"/tmp/$contentId/1617194149349_temp/${fileName.replace(".jpg", ".jpg")}") | ||
val result = new ImageResizerUtil().replace(file.get, newFile) | ||
result.getAbsolutePath should endWith("bitcoin-4_1545114579640.jpg") | ||
result.getAbsolutePath should endWith(fileName.replace(".jpg", ".jpg")) |
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.
The call to fileName.replace(".jpg", ".jpg")
is a no-op. You can simply use fileName
or, if you meant to insert a variant suffix, update the replacement string accordingly.
Copilot uses AI. Check for mistakes.
This PR adds a GitHub Actions workflow for PR code coverage and quality checks to the
asset-enrichment flink job
of Sunbird-Knowlg.PR Code Coverage
This workflow ensures that every proposed change is validated for code quality and coverage before merging.