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.

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

Development: Improve course client test coverage #6491

Merged
merged 6 commits into from
May 1, 2023

Conversation

pal03377
Copy link
Contributor

@pal03377 pal03377 commented Apr 26, 2023

Checklist

General

  • This is a small issue that I tested locally and was confirmed by another developer on a test server.

Client

  • Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data.
  • I followed the coding and design guidelines.
  • I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.

Motivation and Context

#6318 decreased the coverage for some files related to courses. We aim for a line coverage of >90% per file, so this should be improved.

Description

This PR adds tests to improve the coverage on the files from #6318.

Steps for Testing

Check that the tests pass and make sense.

Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Test Coverage

This is the complete table (client only) from #6318 with the updated coverage values. Note that the entries in #6318 apparently were still erroneous in part, so some values here are changed even though this PR does not update the related files.

File Line % Confirmation Improved by this PR
alert.service.ts 91.5%
course-overview.component.ts 90.83%
course-registration.component.ts 93.33%
course-prerequisites-button.component.ts 100%
course-registration-button.component.ts 96.15%
course-registration-detail.component.ts 96.15%

@pal03377 pal03377 added tests client Pull requests that update TypeScript code. (Added Automatically!) labels Apr 26, 2023
@pal03377 pal03377 self-assigned this Apr 26, 2023
pal03377 added a commit that referenced this pull request Apr 26, 2023
@pal03377 pal03377 changed the title Development: Improve course test coverage Development: Improve course client test coverage Apr 26, 2023
@github-actions github-actions bot removed the client Pull requests that update TypeScript code. (Added Automatically!) label Apr 26, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #6491 (0c955e9) into develop (c110bac) will increase coverage by 0.02%.
The diff coverage is 90.90%.

❗ Current head 0c955e9 differs from pull request most recent head b37189a. Consider uploading reports for the commit b37189a to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #6491      +/-   ##
=============================================
+ Coverage      80.49%   80.52%   +0.02%     
- Complexity     13121    13149      +28     
=============================================
  Files           2333     2339       +6     
  Lines          89763    89779      +16     
  Branches       13244    13233      -11     
=============================================
+ Hits           72257    72293      +36     
+ Misses          9645     9630      -15     
+ Partials        7861     7856       -5     
Impacted Files Coverage Δ
...epository/plagiarism/PlagiarismCaseRepository.java 100.00% <ø> (ø)
src/main/webapp/app/entities/exercise.model.ts 76.62% <ø> (ø)
...p/exam/participate/exam-participation.component.ts 67.82% <ø> (-0.19%) ⬇️
...chment-unit-form/attachment-unit-form.component.ts 88.88% <66.66%> (+7.49%) ⬆️
...webapp/app/course/manage/course-storage.service.ts 71.42% <71.42%> (ø)
src/main/webapp/app/overview/courses.component.ts 80.00% <75.00%> (-1.82%) ⬇️
...pp/overview/course-exams/course-exams.component.ts 80.00% <80.00%> (+0.83%) ⬆️
...n/webapp/app/overview/course-overview.component.ts 79.38% <80.00%> (+3.05%) ⬆️
...ebapp/app/shared/constants/score-type.constants.ts 85.71% <85.71%> (ø)
...app/app/course/manage/course-management.service.ts 76.68% <86.66%> (-0.17%) ⬇️
... and 20 more

... and 5 files with indirect coverage changes

Flag Coverage Δ
client 76.53% <89.85%> (+0.03%) ⬆️
server 84.96% <92.68%> (+<0.01%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 335c3d2...b37189a. Read the comment docs.

@pal03377 pal03377 added the client Pull requests that update TypeScript code. (Added Automatically!) label Apr 26, 2023
@github-actions github-actions bot removed the client Pull requests that update TypeScript code. (Added Automatically!) label Apr 26, 2023
@pal03377 pal03377 added client Pull requests that update TypeScript code. (Added Automatically!) ready for review labels Apr 26, 2023
@pal03377 pal03377 marked this pull request as ready for review April 26, 2023 22:00
@pal03377 pal03377 requested a review from a team as a code owner April 26, 2023 22:00
@github-actions github-actions bot removed the client Pull requests that update TypeScript code. (Added Automatically!) label Apr 26, 2023
sven0311
sven0311 previously approved these changes Apr 27, 2023
Copy link
Contributor

@sven0311 sven0311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good 👍

dearjasmina
dearjasmina previously approved these changes Apr 27, 2023
Copy link
Contributor

@dearjasmina dearjasmina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good

lennart-keller
lennart-keller previously approved these changes Apr 28, 2023
Copy link
Contributor

@lennart-keller lennart-keller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks really good.

Copy link
Contributor

@JohannesStoehr JohannesStoehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests look good to me.
One question, is it possible that the subscriptions never actually return anything/are never called actually? I mean that neither () => {...} nor (error) => {...} is invoked?

@krusche krusche added this to the 6.1.6 milestone May 1, 2023
@pal03377 pal03377 dismissed stale reviews from dearjasmina and sven0311 via 28546cf May 1, 2023 20:22
@krusche krusche merged commit 2b2fe95 into develop May 1, 2023
@krusche krusche deleted the chore/test/course-tests-coverage branch May 1, 2023 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants