Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial working set of functional tests based on real streams #225

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IMSoP
Copy link

@IMSoP IMSoP commented Feb 27, 2020

The idea is to build up a library of tests which describe the behaviour
of real file streams under PHP. Regardless of the internal code
structure, this can then be used to check that the functionality of the
virtual file system is as close as possible.

This includes the slightly unusual feature of a mode to test the tests,
by using real files (kind of the opposite of mocking). See the included
README.md for more explanation of the approach.

I'm posting this initial batch of tests as a draft PR to get feedback on
the general concept, and also any thoughts on naming etc. Note also
that a few of these tests don't currently pass on master, mostly because
of the problems discussed #222 which is what prompted me to start
looking into this.

The idea is to build up a library of tests which describe the behaviour
of real file streams under PHP. Regardless of the internal code
structure, this can then be used to check that the functionality of the
virtual file system is as close as possible.
@bizurkur
Copy link
Contributor

bizurkur commented Feb 28, 2020

Seems pretty good to me. I've done something similar in a different project but I used a data provider on each test giving it a stream wrapper path and a real file path. The upside of this approach is normal users won't have to run the real file tests on their system and CI can/will. Whereas in my approach I can't turn off the real file tests.

The only thing I'm curious about and haven't had a chance to test yet is if fwrite should be resetting eof, too. I would hope that if I'm at eof and then write more data that eof would return false.

@IMSoP
Copy link
Author

IMSoP commented Feb 28, 2020

The upside of this approach is normal users won't have to run the real file tests on their system and CI can/will. Whereas in my approach I can't turn off the real file tests.

Yeah, that was the idea: the whole reason this project exists is that testing on the real filesystem is fragile, so I didn't want to make that a pre-requisite. But when you're developing the tests themselves, you can make sure you're measuring the real behaviour, which is sometimes not what you'd guess.

The only thing I'm curious about and haven't had a chance to test yet is if fwrite should be resetting eof, too. I would hope that if I'm at eof and then write more data that eof would return false.

Good call, I'll add some cases in for fwrite+feof in r+ or w+ mode. A nice advantage I've found while writing these is that I can write a test guessing the behaviour, then just run the "test the tests" mode to see if I guessed right.

@mikey179
Copy link
Member

I really like it. I also like the idea of generating code coverage using these tests - maybe we can find some dead code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants