-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
Removed Internet dependency from test e_library_managerTest::testDetectionVersionConsistency()
#4561
base: master
Are you sure you want to change the base?
Conversation
Removed Internet dependency for test `e_library_managerTest::testDetectionVersionConsistency()` And now the test is actually useful because it can now detect the version mismatch between the local copy and the CDN link Fixes: e107inc#4560
e_library_managerTest::testDetectionVersionConsistency()
e_library_managerTest::testDetectionVersionConsistency()
The point of the test was to check that the hard-coded version number located in the library class matched the version number detected and written in the CDN file itself. Hard-coding the version like this improved page-load performance dramatically. It was never about testing local versions of the library against CDN versions of the same library. Perhaps a separate test should be added for this. |
Can we mock the Internet part by loading the local files? It should be out of our scope to check that the CDN is serving the right content, as that behavior is not within our control.
The name of the method |
@CaMer0n, we will continue to have this test failure due to the dependency on the Internet while running "unit" tests: ---------
1) e_library_managerTest: Detection version consistency
Test tests/unit/e_library_managerTest.php:testDetectionVersionConsistency
No looked-up version in core_library:config() -- cdn.fontawesome
Failed asserting that false is true.
#1 /__w/e107/e107/e107_tests/tests/unit/e_library_managerTest.php:84
#2 /__w/e107/e107/e107_tests/vendor/bin/codecept:115 What changes do you need for approval of this pull request? |
Code Climate has analyzed commit dc634d3 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 20.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 34.9% (0.3% change). View more on Code Climate. |
9119ebf
to
2a0ee3f
Compare
Fixes: #4560
Motivation and Context
e_library_managerTest::testDetectionVersionConsistency()
is pointlessly downloading resources from the Internet, so it was really only testing that the CDN was accessible. This is not what a unit test should be doing.Description
Changed the behavior of
e_library_managerTest::testDetectionVersionConsistency()
so that it checks if the CDN library version is the same as the local library version without connecting to the InternetHow Has This Been Tested?
By fixing the existing test so that it actually tests something useful
Types of Changes
Checklist