Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(openchallenges): added unit tests to the image service #2291

Merged
merged 21 commits into from
Nov 3, 2023
Merged

feat(openchallenges): added unit tests to the image service #2291

merged 21 commits into from
Nov 3, 2023

Conversation

mdsage1
Copy link
Contributor

@mdsage1 mdsage1 commented Oct 31, 2023 •
edited
Loading

Description

Updated the unit test to cover api, model/dto and exception. Sonar was not working on my end so I'm unable to confirm the increase in coverage until the pr has been merged.

Related Issue

Add unit test for getting a single organization #1528

[Story] Write a unit test for the enpoint of the image service #1456
feat: Create unit test for the image service #1472

Changelog

  • Add
  1. /workspaces/sage-monorepo/apps/openchallenges/image-service/src/test/java/org/sagebionetworks/exception/ImageHeightNotSpecifiedExceptionTest.java
  2. /workspaces/sage-monorepo/apps/openchallenges/image-service/src/test/java/org/sagebionetworks/exception/SimpleChallengeGlobalExceptionTest.java

@mdsage1 mdsage1 self-assigned this Oct 31, 2023
@mdsage1 mdsage1 changed the title added unit tests for api, model and exception feat: added unit tests for api, model and exception Oct 31, 2023
@mdsage1
Copy link
Contributor Author

mdsage1 commented Oct 31, 2023

@tschaffter I think I've gotten this error before but now that I've deleted the node_modules folder I'm seeing this:

BUILD FAILED in 13s
9 actionable tasks: 9 executed
Failed to execute command: ./gradlew build
Error: Command failed: ./gradlew build
at checkExecSyncError (node:child_process:885:11)
at execSync (node:child_process:957:15)
at runBuilderCommand (/workspaces/sage-monorepo/node_modules/@nxrocks/common/src/lib/core/jvm/utils.js:20:38)
at runBootPluginCommand (/workspaces/sage-monorepo/node_modules/@nxrocks/nx-spring-boot/src/utils/boot-utils.js:18:43)
at /workspaces/sage-monorepo/node_modules/@nxrocks/nx-spring-boot/src/executors/build/executor.js:10:54
at Generator.next ()
at /workspaces/sage-monorepo/node_modules/tslib/tslib.js:118:75
at new Promise ()
at Object.__awaiter (/workspaces/sage-monorepo/node_modules/tslib/tslib.js:114:16)
at buildExecutor (/workspaces/sage-monorepo/node_modules/@nxrocks/nx-spring-boot/src/executors/build/executor.js:8:20)
at /workspaces/sage-monorepo/node_modules/nx/src/command-line/run/run.js:81:23
at Generator.next ()
at fulfilled (/workspaces/sage-monorepo/node_modules/nx/node_modules/tslib/tslib.js:166:62)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Can you confirm that it's unrelated to node_modules and that I need to check my unit test code, please?

@tschaffter
Copy link
Member

I've deleted the node_modules folder

Did you recreate the folder with workspace-install? Even if it includes Node.js packages and you are working on a Java project, there are Node.js packages that are required for the monorepo to work, e.g. nx.

I believe that the error message is reported by a git hook but the error effectively comes from when gradlew build. Can you access the real error message thrown by gradle, e.g. by building the project that fail?

mdsage1 and others added 15 commits November 1, 2023 17:11
@mdsage1
remove files
f9005a3
@mdsage1
update istest
2192727
@tschaffter @mdsage1
Update label of the Discord button (Sage-Bionetworks#2288)
1073801
@vpchung @mdsage1
fix(openchallenges): upgrade legacy components (Sage-Bionetworks#2290)
b3d8da4 
* not-found page: upgrade MatCard

* org-profile page: remove unused tabs component

* github-button: upgrade MatButton

* _app-theme: upgrade legacy; fix fonts

* navbar: upgrade MatButton; fix colors and padding

* fix typography for buttons

* signup page: upgrade components

* add required `mat-label`

* update styles to account for changes

* user-button: upgrade MatButton, MatMenu

* update styles to account for changes

* upgrade component in commented code

* remove dependency on legacy-* in _app-theme

* add typography to discord button; add discord theme

* schematic: remove unused font; upgrade legacies
@vpchung @mdsage1
fix(openchallenges): remove Sage from sponsor list (Sage-Bionetworks#…
282191d 
…2287)

* remove Sage from sponsor list

* apply section-title styling

* link out to ITCR website

* use more descriptive variable name
@tschaffter @mdsage1
chore(schematic): remove schematic-app and `schematic-api-client-an…
9e7baef 
…gular` (Sage-Bionetworks#2293)

* Remove schematic-app and schematic-api-client-angular

* Update `tsconfig.base.json`
@tschaffter @mdsage1
feat(sage-monorepo): enable Format on Save for JSON and JSON with Com…
b0bef99 
…ments (Sage-Bionetworks#2294)

* Format JSON on save

* Enable format on save for jsonc
@rrchai @mdsage1
hide page size options on search pages (Sage-Bionetworks#2298)
e22874d
@mdsage1
delete
996222e
@mdsage1
Merge branch 'Sage-Bionetworks:main' into img_srv_unittest_cvg_incr1
8e3ae46
@mdsage1
img hgt exception test
b21e2be
@mdsage1
final image height exception test
e035e39
@mdsage1
update
49d7f1c
@mdsage1
additional variable formatting
db67dbf
@mdsage1
updtate unittest
d5124ca
@mdsage1 mdsage1 changed the title feat: added unit tests for api, model and exception feat: added unit tests for exception Nov 1, 2023
@mdsage1 mdsage1 marked this pull request as ready for review November 1, 2023 22:08
@mdsage1
Copy link
Contributor Author

mdsage1 commented Nov 1, 2023

@tschaffter I added changes to unit tests for exception section of the project.

@tschaffter
Copy link
Member

What is the coverage percentage for the two new files from the HTML report?

@mdsage1
Copy link
Contributor Author

mdsage1 commented Nov 2, 2023 •
edited
Loading

What is the coverage percentage for the two new files from the HTML report?

@tschaffter I was unable to see any changes so I went ahead and created the unit tests. I wrote it as a comment in the Task and forgot to write it here. I'm hoping to be able to see the changes in coverage once the unit tests are synced to the main branch.

@tschaffter tschaffter self-requested a review November 3, 2023 15:21
@tschaffter
Copy link
Member

tschaffter commented Nov 3, 2023 •
edited
Loading

@mdsage1 The coverage report is generated properly on my end. This PR brings the coverage of the image service to 30% (see screenshot).

Regarding the Java classes generated by OpenAPI Generator, there are two ways we could handle them (future work):

  1. Check if OpenAPI Generator has an option to ask it to generate tests for the classes it generates.
  2. Ignore these classes as discussed. If we can specify to the test runner that these classes should not be covered, the coverage percent would increase.

I'll approve and merge this PR.

Coverage

Global

image

Exception package

image

@tschaffter tschaffter changed the title feat: added unit tests for exception feat(openchallenges): added unit tests to the image service Nov 3, 2023
@tschaffter tschaffter merged commit e2bdab8 into Sage-Bionetworks:main Nov 3, 2023
7 checks passed
This was referenced Nov 3, 2023
Closed
Merged
@mdsage1 mdsage1 deleted the img_srv_unittest_cvg_incr1 branch February 16, 2024 16:49
@mdsage1 mdsage1 restored the img_srv_unittest_cvg_incr1 branch February 16, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rrchai rrchai Awaiting requested review from rrchai rrchai is a code owner

@vpchung vpchung Awaiting requested review from vpchung

@tschaffter tschaffter Awaiting requested review from tschaffter tschaffter is a code owner

Assignees

@mdsage1 mdsage1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mdsage1 @tschaffter @vpchung @rrchai