Opened 9 years ago
Closed 9 years ago
#37523 closed enhancement (fixed)
Unit tests: Move classes out of includes/functions.php into their own file
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Build/Test Tools | Keywords: | good-first-bug has-patch needs-testing |
Focuses: | Cc: |
Description
The includes/functions.php
file includes two classes, Basic_Object
and Basic_Subclass
. The attached patch moves these functions into their own file, and includes them in tests/basic.php
.
Attachments (3)
Change History (14)
#1
@
9 years ago
- Milestone changed from Awaiting Review to 4.7
- Owner set to boonebgorges
- Status changed from new to assigned
#3
follow-up:
↓ 4
@
9 years ago
This broke unit tests that I have that use the Basic_Object
class in a test since they are now only loaded in that test file instead of the functions file. Could the require calls be moved to the functions file so those classes are always available?
#4
in reply to:
↑ 3
@
9 years ago
- Keywords needs-patch added
- Resolution fixed deleted
- Status changed from closed to reopened
Replying to TimothyBlynJacobs:
This broke unit tests that I have that use the
Basic_Object
class in a test since they are now only loaded in that test file instead of the functions file. Could the require calls be moved to the functions file so those classes are always available?
Yes.
#7
follow-up:
↓ 8
@
9 years ago
I think it would be cleaner to load the files during the bootstrap process, instead of including them in functions.php
. Nothing in this file relies on the two classes, but the tests do.
There's a small typo in the patch. It's in the word UnitTests in the documentation header of class-basic-object.php
.
@
9 years ago
New Revision of the previous patch file I submitted. With the changes suggested by @Frank Klein. I moved the including of the classes to the bootstrap.php file instead. Below the inclusion of the funtions.php file as no reference is made to the classes up to this point. Did not test this, however it seems like the revision would run as expected
#8
in reply to:
↑ 7
@
9 years ago
Replying to Frank Klein:
I think it would be cleaner to load the files during the bootstrap process, instead of including them in
functions.php
. Nothing in this file relies on the two classes, but the tests do.
There's a small typo in the patch. It's in the word UnitTests in the documentation header of
class-basic-object.php
.
Thanks for pointing that out. I have revised my patch and submitted it.
Apologies for the typo on that one.
#9
@
9 years ago
- Status changed from reopened to assigned
Thanks for the patches. I'm inclined to leave the require_once
statements in functions.php, given that:
- our main goal is backward compatibility, and functions.php is where the classes were previously loaded in the call graph
- none of this will matter once we autoload classes; see #36335
@Frank Klein Thanks for the patch!