Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Adding some basic tests would aid development. Patch to follow.
Comment | File | Size | Author |
---|---|---|---|
#7 | browscap-add_tests-2429207-7.patch | 8.85 KB | AohRveTPV |
#1 | browscap-add_tests-2429207-1.patch | 1.92 KB | AohRveTPV |
Comments
Comment #1
AohRveTPV CreditAttribution: AohRveTPV commentedThis test is probably not suitable for use by Testbot since it downloads from browscap.org. Frequent browscap.org downloads cause an IP address ban. It should be useful for local testing, though.
If it were possible to configure the data URL (#1788720: Allow to change the URLs to use for importing useragent information), the URL could maybe be changed to that of a sample local INI file for testing. Or maybe SimpleTest has some sort of intercepting proxy capability.
Comment #2
gregglesThis seems like a great start - thanks so much!
I wonder if we could pick out a handful of lines from the browscap file that are necessary as a test fixture and just manually insert those lines (or, ideally, have a really lightweight file that we import so that we can test some of the importing stuff too!). That would let us run these tests on d.o without worrying about the ip ban and let us run the tests everywhere in a smaller amount of time.
Anyway, that's a minor improvement that shouldn't block progress on this :)
The first time I tested it I got an error because the import timed out my apache process :(
So, I tested it with #2305223: Optimize database access in _browscap_import() applied and the tests all pass. Yay!
Comment #3
AohRveTPV CreditAttribution: AohRveTPV commentedI am working on this.
It may be good to still also have a test that checks for and downloads the actual Browscap data. The SimpleTest examples had a
isRunningOnTestbot()
method that would be useful, but it does not seem to be present in Drupal 7 built-in SimpleTest.Comment #4
AohRveTPV CreditAttribution: AohRveTPV commentedChanged to use local sample INI file for testing. It is read instead of downloading from browscap.org when the
browscap_test_mode
variable is set to TRUE (default forbrowscap.test
).To facilitate substituting parts of
_browscap_import()
for testing, I broke it into several sub-functions.The sample INI file was created by taking the latest file and removing every user agents section except for IE 9. (Properties inheritance should still be tested by this small file, since the IE 9 user agent inherits from DefaultProperties.) The test user agent string was chosen arbitrarily.
Please review and see if this is a good approach.
Comment #5
AohRveTPV CreditAttribution: AohRveTPV commentedgetInfo()
description should not be translated.Comment #6
AohRveTPV CreditAttribution: AohRveTPV commented- Remove
t()
enclosinggetInfo()
description.- Change docblocks for
getInfo()
andsetUp()
.- Add blank line between last method of class and closing brace per coding standards.
Comment #7
AohRveTPV CreditAttribution: AohRveTPV commentedComment #8
AstonVictor CreditAttribution: AstonVictor at DevBranch commentedI'm closing it because the issue was created a long time ago without any further steps.
if you still need it then raise a new one.
thanks