WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#35976 closed enhancement (fixed)

Unit test: Check if the device can upload

Reported by: borgesbruno Owned by: johnbillion
Milestone: 4.5 Priority: normal
Severity: normal Version:
Component: Upload Keywords: needs-patch
Focuses: Cc:

Description

I've wrote some tests for the function that check whether a device can upload. The tests cover many devices through a small list of the user agents.

Attachments (2)

35976.diff (1.9 KB) - added by borgesbruno 6 years ago.
35976_refactored.diff (2.2 KB) - added by borgesbruno 6 years ago.
Code refactor, added comments

Download all attachments as: .zip

Change History (13)

@borgesbruno
6 years ago

#1 @borgesbruno
6 years ago

  • Keywords has-patch has-unit-tests added

#2 @johnbillion
6 years ago

  • Component changed from General to Upload

Thanks @borgesbruno.

Can you add an inline comment to each of the datas in the dataProvider stating which browser the user agent refers to? This will make maintenance, debugging, etc easier.

@borgesbruno
6 years ago

Code refactor, added comments

#3 @borgesbruno
6 years ago

Hey @johnbillion, tks for the review, it's done ;-)

#4 @borgesbruno
6 years ago

  • Keywords dev-feedback added

#5 @borgesbruno
6 years ago

  • Keywords dev-feedback removed

#6 @johnbillion
6 years ago

  • Milestone changed from Awaiting Review to 4.5

#7 @johnbillion
6 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 36810:

Uploads: Add tests for device upload capabilities based on user agent.

Fixes #35976
Props borgesbruno

#8 @johnbillion
6 years ago

  • Keywords needs-patch added; has-patch has-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Tests are failing due to wp_is_mobile() being called indirectly in an earlier test, which sets its $is_mobile static var to false, which causes _device_can_upload() to always return true.

Running the test_device_can_upload() test in isolation succeeds.

The solution is to remove the somewhat pointless static var in wp_is_mobile(), which only serves to avoid a handful of strpos() calls which are fast anyway.

Last edited 6 years ago by johnbillion (previous) (diff)

#9 @johnbillion
6 years ago

In 36813:

Uploads: Remove an unnecessary static var from wp_is_mobile() to allow its direct and indirect use within unit tests. The static `$is_m
obile var was only used to avoid a handful of calls to strpos()`, which are exceptionally fast and result in no measurable increase in
processing time on each call to wp_is_mobile().

See #35976, #20014

Last edited 6 years ago by johnbillion (previous) (diff)

#10 @johnbillion
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

#11 @borgesbruno
6 years ago

Great @johnbillion! Tks for the additional patch

Note: See TracTickets for help on using tickets.