This repository was archived by the owner on Nov 24, 2025. It is now read-only.
TPv2 Testing improvements#6513
Merged
shamrickus merged 50 commits intoapache:masterfrom Feb 9, 2022
Merged
Conversation
(cherry picked from commit 699ff7232593a74d90c0856f4224e0197087efd4)
(cherry picked from commit b3f8fa87bcbf96431e60637215405376e6680e26)
(Replaced by Angular Material MatDialog) (cherry picked from commit 593a04ce36334ae9e60c6e6580dd4532476279bf)
(cherry picked from commit 73891df8e6368d1d5801d93b5d7ef7a537aaa890)
and also putting newlines between import groups and alphabetization within groups finally, importing from "../../**/*" is no longer allowed.
Also the tests revealed a shortcoming of the directive where it failed to properly implement the behavior of the browser-native `HTMLObjectElement.setCustomValidity` where passing an empty string marked the customError part of its validity state as non-erroneous.
Also made the header's `logout` method asynchronous for ease of use.
This also helped me fix a bug - the component was unable to successfully update any server's or servers' status(es) because it was passing a Status ID instead of a Name. Also made the submit handler async because that's much easier to deal with.
I also simplified a couple of methods by making them asynchronous
Also updated the actual component to use an async method to handle form submission, for ease-of-use.
This also uncovered a bug in rendering HTTP_ONLY protocol strings, which is now fixed.
Also got rid of an unused property
c126779 to
4e7a015
Compare
I enabled that rule just for myself going through the project so that I could update things as I went along but because the rule isn't Angular-aware, it's emitting warnings for types used for dependency injection, which breaks compilation if 'fixed' (e.g. with 'ng lint --fix').
Has better support for Angular concepts. Also, is now an optional dependency rather than a development dependency.
9f311ff to
ff4d856
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Primarily this PR improves unit testing coverage by a ton, from
to
Part of how it does this is by separating out the API services into their own module, with a testing module that provides mock services. Prior to this, a running Traffic Ops instance would have been required to run the unit tests written for this PR - this is no longer the case, and should never again be the case in the future.
It's also true that, because of this, the API services are not under test (not that they were before these changes), and are not included in the coverage statistics. At some point, any logic beyond sending HTTP requests probably ought to be extracted from those files to both further enhance test coverage as well as give a more accurate coverage report.
This also fixes an issue where coverage could not be generated due to using a legacy plugin for which support has been dropped, updates Angular to version 13.2.0, and reduces
npm audit-found vulnerabilities from40 vulnerabilities (2 low, 35 moderate, 3 high)to literally 0.I've also added four NPM scripts -
doc, which generates documentation for the project usingcompodoc(optional dependency),doc:servewhich serves the documentation over HTTP on localhost,coveragewhich runs the unit tests and outputs coverage information, andcoverage:ciwhich runs the unit tests without watching for changes and in a headless browser and outputs coverage information.Finally, during the writing of tests a few bugs were uncovered and subsequently fixed.
Which Traffic Control components are affected by this PR?
None
What is the best way to verify this PR?
Make sure all the new tests pass, check out the code coverage to verify it if you want (running the tests with coverage output will fail on master for the reasons I specified above, so you'd need to install the correct karma plugin and update configuration accordingly to get it to work).
PR submission checklist