Bug 72031 – [Qt] Skip tests when no network is present

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Bruno Abinader (history only)]

Some benchmark tests requires network connection (they fetch data from external URL's) and so they fail if no network is present. These tests should be skipped instead.

These are the tests that are currently failing when no network is present:

- tst_Loading::load
- tst_Painting::paint

[Bruno Abinader (history only)]

Created attachment 114512 [details]
Proposed patch (verifies for network connection - if not, skip the test).

Proposed patch that verifies if a network connection is present, otherwise skip the tests.

[WebKit Review Bot]

Attachment 114512 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/ChangeLog', u'Source/WebK..." exit_code: 1

Source/WebKit/qt/tests/benchmarks/painting/tst_painting.cpp:22:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/qt/tests/benchmarks/painting/tst_painting.cpp:112:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebKit/qt/tests/benchmarks/loading/tst_loading.cpp:22:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/qt/tests/benchmarks/loading/tst_loading.cpp:113:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 4 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 114512 [details]
Proposed patch (verifies for network connection - if not, skip the test).

Attachment 114512 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10394359

[Bruno Abinader (history only)]

Created attachment 114516 [details]
Updated patch with fixes mentioned by build bot.

Fixed issues found by style check.

[Bruno Abinader (history only)]

Created attachment 114517 [details]
Updated patch with missing m_manager class member.

Added missing m_manager class member (oops!).

[Early Warning System Bot]

Comment on attachment 114517 [details]
Updated patch with missing m_manager class member.

Attachment 114517 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10395359

[Bruno Abinader (history only)]

Created attachment 114520 [details]
Updated patch which fixes method signature typo.

Fixed method signature typo (oops!).

[Simon Hausmann]

Comment on attachment 114520 [details]
Updated patch which fixes method signature typo.

I think your patch looks good in general, but I also think that you can simplify it: Just allocate a QNetworkConfigurationManager instance directly as member variable. Then you don't need to allocate it on the heap and you don't need to deal with it in initTestCase().

QNetworkConfigurationManager itself is a pretty lightweight class that just forwards signals for the actual private singleton. There's no need to allocate it on the heap :)

[Bruno Abinader (history only)]

Created attachment 114644 [details]
Updated patch with memory allocation changes reviewed by Simon.

Good point Simon, I always tend to allocate objects on the heap while doing unit tests so I can delete them on a specific moment if needed. But in this case, as you said, it is not needed at all.

[Simon Hausmann]

Comment on attachment 114644 [details]
Updated patch with memory allocation changes reviewed by Simon.

r=me

[WebKit Review Bot]

Comment on attachment 114644 [details]
Updated patch with memory allocation changes reviewed by Simon.

Clearing flags on attachment: 114644

Committed r100053: <http://trac.webkit.org/changeset/100053>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.