Adding some basic tests would aid development. Patch to follow.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

AohRveTPV’s picture

Status: Active » Needs review
FileSize
1.92 KB

This 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.

greggles’s picture

This 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 :(

POST http://www.d7.dev/admin/config/system/browscap returned 200 (0 bytes).	

So, I tested it with #2305223: Optimize database access in _browscap_import() applied and the tests all pass. Yay!

AohRveTPV’s picture

Status: Needs review » Needs work

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.

I 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.

AohRveTPV’s picture

Status: Needs work » Needs review
FileSize
32.78 KB

Changed 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 for browscap.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.

AohRveTPV’s picture

Status: Needs review » Needs work

getInfo() description should not be translated.

AohRveTPV’s picture

Status: Needs work » Needs review

- Remove t() enclosing getInfo() description.
- Change docblocks for getInfo() and setUp().
- Add blank line between last method of class and closing brace per coding standards.

AohRveTPV’s picture

FileSize
8.85 KB
AstonVictor’s picture

Status: Needs review » Closed (outdated)

I'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