Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Conversation

@ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented Mar 24, 2023

This PR adds 100% test coverage to the API services in TPv2, primarily. It also adds an overload signature or two, some overload JSDoc comments, and fixes a couple bugs I encountered while writing the tests.


Which Traffic Control components are affected by this PR?

  • Traffic Portal (experimental v2)

What is the best way to verify this PR?

Make sure the new unit tests pass. Verify 100% coverage with ng test --code-coverage (note the testing services have no such promise of coverage (yet)).

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR doesn't need a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added low impact affects only a small portion of a CDN, and cannot itself break one tests related to tests and/or testing infrastructure tech debt rework due to choosing easy/limited solution experimental a feature/component not directly supported by ATC Traffic Portal v2 Related to the experimental Traffic Portal version 2 labels Mar 24, 2023
@shamrickus shamrickus self-assigned this Apr 10, 2023
@shamrickus
Copy link
Member

This will need to be rebased as next time the E2E tests will fail due to chrome version mismatch

@ocket8888 ocket8888 force-pushed the tpv2/api-tech-debt branch from 17fb49b to e7855fa Compare April 24, 2023 23:29
Copy link
Member

@shamrickus shamrickus left a comment

Choose a reason for hiding this comment

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

LGTM

Also improved base class test coverage to 100%
… to delete divisions directly

Also expanded test coverage of the CG service to 100%
Also simplify an operation in the DS component where only data is
required
Also added the ability to directly delete a Job instead of requiring the
caller to dereference that for us. Also improved test coverage to 100%.
…nger necessary)

Also added 100% test coverage
…onses

Also fixed a bug where it was impossible to get a single status.
Also also added 100% test coverage and JSDocs for all of the overload
signatures.
Also add 100% test coverage
… user

also removed the "ability" to get a Role by ID - which is something
Roles no longer have. Also also added JSDoc comments for all overloads.
Also also also added 100% test coverage.
@ocket8888 ocket8888 force-pushed the tpv2/api-tech-debt branch from e7855fa to 680c102 Compare April 28, 2023 19:47
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #7421 (680c102) into master (39f4019) will increase coverage by 4.32%.
The diff coverage is 98.37%.

@@             Coverage Diff              @@
##             master    #7421      +/-   ##
============================================
+ Coverage     61.11%   65.44%   +4.32%     
  Complexity       98       98              
============================================
  Files           305      305              
  Lines         11674    11616      -58     
  Branches        795      788       -7     
============================================
+ Hits           7135     7602     +467     
+ Misses         4191     3667     -524     
+ Partials        348      347       -1     
Flag Coverage Δ
traffic_portal_v2 76.44% <98.37%> (+12.13%) ⬆️
unit_tests 76.44% <98.37%> (+12.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...traffic-portal/src/app/api/testing/user.service.ts 46.87% <0.00%> (+1.68%) ⬆️
...tal/traffic-portal/src/app/api/base-api.service.ts 100.00% <100.00%> (+41.17%) ⬆️
.../traffic-portal/src/app/api/cache-group.service.ts 100.00% <100.00%> (+99.09%) ⬆️
...rimental/traffic-portal/src/app/api/cdn.service.ts 100.00% <100.00%> (+96.00%) ⬆️
.../traffic-portal/src/app/api/change-logs.service.ts 100.00% <100.00%> (+80.00%) ⬆️
...fic-portal/src/app/api/delivery-service.service.ts 100.00% <100.00%> (+99.20%) ⬆️
...fic-portal/src/app/api/invalidation-job.service.ts 100.00% <100.00%> (+96.00%) ⬆️
...ic-portal/src/app/api/physical-location.service.ts 100.00% <100.00%> (+95.23%) ⬆️
...ntal/traffic-portal/src/app/api/profile.service.ts 100.00% <100.00%> (+94.44%) ⬆️
...ental/traffic-portal/src/app/api/server.service.ts 100.00% <100.00%> (+98.48%) ⬆️
... and 4 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@shamrickus shamrickus merged commit 6d6876c into apache:master May 2, 2023
@ocket8888 ocket8888 deleted the tpv2/api-tech-debt branch May 2, 2023 16:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

experimental a feature/component not directly supported by ATC low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution tests related to tests and/or testing infrastructure Traffic Portal v2 Related to the experimental Traffic Portal version 2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants