Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#25784 closed enhancement (fixed)

shortcode.js needs unit tests

Reported by: iandunn's profile iandunn Owned by:
Milestone: 3.8 Priority: normal
Severity: normal Version: 3.8
Component: Unit Tests Keywords: has-patch
Focuses: Cc:

Description

Part of the ongoing effort to increase test coverage.

Attachments (2)

25784.diff (5.6 KB) - added by iandunn 11 years ago.
25784.2.diff (7.0 KB) - added by jorbin 10 years ago.

Download all attachments as: .zip

Change History (6)

@iandunn
11 years ago

@jorbin
10 years ago

#1 follow-up: @jorbin
10 years ago

  • Milestone changed from Awaiting Review to 3.8

Great start Iandunn!

I cleaned the file up a bit based on our new jshnit standards (which were added after your initial patch).

I also divided the assertions up a bit more. I think it's better to have more tests with a few direct assertions than tests with a lot of assertions. It makes it much easier to track down specific regressions.

#2 @nacin
10 years ago

In 26222:

Add JavaScript tests for shortcode.js.

props jorbin, iandunn.
see #25784.

#3 in reply to: ↑ 1 @iandunn
10 years ago

Replying to jorbin:

I cleaned the file up a bit based on our new jshnit standards (which were added after your initial patch).

Awesome, thanks :)

I also divided the assertions up a bit more. I think it's better to have more tests with a few direct assertions than tests with a lot of assertions. It makes it much easier to track down specific regressions.

Yeah, outside of Core I'm used to limiting each function to a single assertion, but on another ticket Sergey recommended grouping them. I think I probably grouped them a bit too much, though, so I like your version better.

#4 @nacin
10 years ago

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

New tickets for new tests; going to close this one.

Note: See TracTickets for help on using tickets.