-
Notifications
You must be signed in to change notification settings - Fork 61
Unit tests written for Improving the Coverage #295
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #295 +/- ##
===========================================
+ Coverage 50.62% 65.20% +14.58%
===========================================
Files 66 66
Lines 3771 3771
Branches 444 444
===========================================
+ Hits 1909 2459 +550
+ Misses 1767 1186 -581
- Partials 95 126 +31
Continue to review full report at Codecov.
|
@Sanji515 SIr, Can you please take a look at the unit tests i have written. I am not sure whether i am doing it correctly or not as i am just a begineer in writing tests. If it's fine i'll continue writing tests for the other services and components and if not then i will work on improving the tests i have written till now. |
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.
From next time try to make small PRs - a specific code change , so that we can review it quickly. For now it is fine.
Codecov Report
@@ Coverage Diff @@
## master #295 +/- ##
===========================================
+ Coverage 50.16% 64.47% +14.30%
===========================================
Files 66 66
Lines 3873 3873
Branches 450 450
===========================================
+ Hits 1943 2497 +554
+ Misses 1836 1251 -585
- Partials 94 125 +31
Continue to review full report at Codecov.
|
@@ -53,4 +53,12 @@ describe('AnalyticsComponent', () => { | |||
it('should create', () => { | |||
expect(component).toBeTruthy(); | |||
}); | |||
|
|||
it('should call ngOninit', () => { |
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.
Is it possible to test more lines than the trivial component cases?
expect(service.getUrl).toHaveBeenCalled(); | ||
expect(service3.teamCountAnalyticsURL).toHaveBeenCalled(); | ||
})); | ||
it('should show download challenge participant team', inject([EndpointsService, ApiService], |
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.
it('should show download challenge participant team', inject([EndpointsService, ApiService], | |
it('should download challenge participant team', inject([EndpointsService, ApiService], |
expect(endpointsService.updateChallengePhaseDetailsURL(2, 3)).toBe('challenges/challenge/2/challenge_phase/3'); | ||
}); | ||
it('should return challenge phase split url' , () => { | ||
expect(endpointsService.challengePhaseSplitURL(2)).toBe('challenges/2/challenge_phase_split'); |
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.
Whenever possible, try not to hardcode URLs. Make them variables at the top and use them.
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.
Overall, great job @Kajol-Kumari! At some places, I felt more lines in the code could be tested and we might boost the test coverage a little more. Since this is not the top priority for now, you could add more tests a little later when you can.
2bf4cb0
to
d59fe7d
Compare
@Kajol-Kumari Can you please check why the build is failing here? |
…improve_coverage_ngx
68805ee
to
3dcb306
Compare
…improve_coverage_ngx
9d4a90a
to
28f265f
Compare
Feat: #85
Feature: Unit Tests for written for improving coverage for following files: