Fix skipped/incomplete core tests for 3.3 series #552
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This PR is not ready to merge, it requires #548 and a rebase first |
|
Excellent work @acoulton ! |
acoulton
force-pushed
the
3.3/bug/reinstate-skipped-tests
branch
2 times, most recently
from
65d2131 to
6082568
Compare
|
This is now ready for review and merge - all 18 skipped/incomplete tests are now green. |
The cookie tests were all being skipped because headers are always sent by the PHPUnit test runner (fixes kohana/kohana#47). Additionally even when they ran they were covering very little of the actual logic of the class, mostly just asserting that setcookie returns TRUE. Rewrote the tests completely and slightly refactored the Cookie class to allow mocking of the timestamp and setcookie calls. Refs #547.
These tests were disabled in 46e2ee9 because they were generating an error if headers had already been sent. http://dev.kohanaframework.org/issues/4155 This appears no longer the case - presumably the Session class or test configuration/environment has changed since then. The tests could probably still be clearer and more comprehensive, but they run.
These tests aren't great, and the PHP mime-type test is particularly likely to be unreliable. We should be able to get a valid (eg image/png) result in all current environments though for the moment. Probably the File::mime method would be best reimplemented with some third-party component, surely this is a solved problem by now.
These tests were vulnerable to breakage if modules provide their own validation message files, so were being skipped. Fixed to use (hopefully) collision-proof message files with known values and to cover a broader range of cases including missing file and missing key. Would ideally be further refactored to mock/have complete control of the module search path outside global state.
Original test was being skipped because it was hitting the internet - the major relevant functionality here is the parsing rather than the remote access. Added local example feeds for RSS2.0 as well as the previously tested atom feed. Also improved the tests to check for the title as well as the number of elements since we now have known content to work with.
This test was originally implemented in bf0175f to fix http://dev.kohanaframework.org/issues/3767 - it was required because headers were sent directly by the response class, and once they had been sent it was then very difficult to test other variations. The fact that headers are not relevant in CLI appears to have been a side concern. Subsequently in a1943b0 header sending moved to HTTP_Header which mocks out `header` with a callback interface for testing instead. Since that refactoring (released in v3.2.0) the headers are always sent in CLI unless headers_sent is already true. Therefore this test is invalid and can be removed.
acoulton
force-pushed
the
3.3/bug/reinstate-skipped-tests
branch
from
6082568 to
1631e0b
Compare
enov
added a commit
that referenced
this pull request
Sep 23, 2014
Fix skipped/incomplete core tests for 3.3 series
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull reinstates and improves tests that were being skipped on CI as discussed in #547, in particular:
There are now no skipped tests on kohana/core.
NB that this PR depends on the changes in #548, once that's merged I'll rebase this.
@enov are you happy that the refactorings here are all safe to release in the 3.3 bugfix series?