Closed
Conversation
yadij
force-pushed
the
cleanup-unit-tests
branch
from
aaab83d to
a3d36fb
Compare
rousskov
added
the
S-waiting-for-committer
yadij
force-pushed
the
cleanup-unit-tests
branch
from
a3d36fb to
4a30553
Compare
It does not actually need anything in TESTSOURCES and libbase is not built until after libmem so having a test dependency did not make sense.
This test does not need anything from TESTSOURCES. Removing that also removes the need for stubs.
This test does not need anything from TESTSOURCES. Removing that also removes the need for stubs. Also, since this is a template file we do not even need libbase linked to test its functionality.
This test does not need anything from TESTSOURCES. Removing that also removes the need for several stubs. TODO: - Make LookupTable able to be used with other types than SBuf. eg. std::string would simplify the tests further and avoid SBuf bugs from impacting this unit test.
* as a template it does not need libbase to be linked. This also removes the need for several stub files. * fix RefCount.h header not distributed properly by base/Makefile. This was exposed by use of nodist_ in the unit test.
* add stub files for HttpHeader.cc and http/libhttp.la
Symbols defined in store/libstore.la should be defined in a library specific stub rather than spread through other stub files related to store classes and files. stub_store.cc is for objects in the src/store.cc file. Some removal policy symbols are still incorrectly stub'ed there. For now just extracting the libstore.la symbols. Fixes the mis-naming of stub_SwapDir for Store::Disk symbols. SwapDir is a typedef alias not a class or filename.
yadij
force-pushed
the
cleanup-unit-tests
branch
from
4a30553 to
2146d95
Compare
Contributor
Author
|
Removing the WIP, this can be applied without completing all test pruning. Most of the remaining pruning is blocked by storeAppend*() API dependency - for which PR #143 was the optimal fix. |
Contributor
FYI: The WIP prefix is still in this PR title. |
yadij
changed the title
Contributor
Author
|
@rousskov Do you wish to review any of this? or shall I clear Anubis to proceed? |
kinkie
approved these changes
Jan 23, 2019
Contributor
kinkie
left a comment
kinkie
left a comment
There was a problem hiding this comment.
As testing works and documentation is update, LGTM
Contributor
No, I do not. I would not have added an |
yadij
added
M-cleared-for-merge
squid-anubis
pushed a commit
that referenced
this pull request
Jan 25, 2019
Polish unit tests to reduce their dependency lists inline with the updated test documentation requirements. Some further cleanup of unit tests documentation based on experience pruning existing unit tests. * Only the test logic files actually need to be distributed by a test. All files being tested are supposed to be distributed from elsewhere and the test should rely on that to prevent future issues like the base/RefCount.h bug mentioned below. * Starting to deprecate TESTSOURCES which is causing more trouble than benefit by pulling in too many needless dependencies. eg the globals.cc objects and symbols. Tools it used to provide are largely superseded by the stub mechanisms. Add missing stub files necessary for pruning dependencies. Also fix some bugs in existing stub files. Fix base/RefCount.h distribution. This file was not included in the base/libbase.la dependency list but indirectly being distributed by the unit-test existence. Which in some builds could cause it not to exist in generated minimal tarballs.
squid-anubis
added
M-waiting-staging-checks
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Polish unit tests to reduce their dependency lists inline with
the updated test documentation requirements.
Some further cleanup of unit tests documentation based on
experience pruning existing unit tests.
Only the test logic files actually need to be distributed by
a test. All files being tested are supposed to be distributed
from elsewhere and the test should rely on that to prevent
future issues like the base/RefCount.h bug mentioned below.
Starting to deprecate TESTSOURCES which is causing more
trouble than benefit by pulling in too many needless
dependencies. eg the globals.cc objects and symbols. Tools it
used to provide are largely superseded by the stub mechanisms.
Add missing stub files necessary for pruning dependencies. Also
fix some bugs in existing stub files.
Fix base/RefCount.h distribution. This file was not included in
the base/libbase.la dependency list but indirectly being
distributed by the unit-test existence. Which in some builds
could cause it not to exist in generated minimal tarballs.