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

Double open write, write/truncate/flush #302

Closed
agroce opened this issue Sep 21, 2017 · 23 comments
Closed

Double open write, write/truncate/flush #302

agroce opened this issue Sep 21, 2017 · 23 comments
Labels

Comments

@agroce
Copy link

agroce commented Sep 21, 2017

import pyfakefs.fake_filesystem
import os

fs = pyfakefs.fake_filesystem.FakeFilesystem()
fs.CreateDirectory('/Volumes')
fs.CreateDirectory('/Volumes/ramdisk')
fs.CreateDirectory('/Volumes/ramdisk/test')
os0 = pyfakefs.fake_filesystem.FakeOsModule(fs)
opener0 = pyfakefs.fake_filesystem.FakeFileOpen(fs)

component0 = "alpha"
path0 = "/Volumes/ramdisk/test"
path0 += "/" + component0
str0 = ""
str0 += 'a'
str0 += 'a'
str0 += 'a'
str0 += 'a'
str0 += 'a'
str0 += 'a'
file0 = opener0(path0,'w')
file1 = opener0(path0,'w')
file0.write(str0)
file0.truncate()
file1.flush()
result = os0.path.getsize(path0)

print result

pyfakefs 0

OS 6

@mrbean-bremen
Copy link
Member

Hm, wouldn't have expected this, this buffering stuff is tricky. For today I'm done, maybe will tackle this tomorrow .

@agroce
Copy link
Author

agroce commented Sep 21, 2017

Yeah, I found the OS behavior weird and pyfakefs totally reasonable, but this stuff is obviously tricky. Is this all coming from POSIX, or what, the buffering "rules"?

@mrbean-bremen
Copy link
Member

I don't think that POSIX defines the specific buffering behavior, other than the fact that there is a buffering layer. This is up to the OS and/or Python. I haven't checked, but from what I have seen in the Python code, it pretty much relies on standard OS behavior. Anyway, we better implement the weird behavior in pyfakefs...

@mrbean-bremen
Copy link
Member

@agroce : I started trying to fix this issue and found at least one existing test not matching reality, so I started rewriting some of the tests to run both on pyfakefs and the real file system to get a better picture which tests are incorrect (I told you that I wanted this in some other thread). I found quite some of tests that are not correct under Windows and/or Linux, so I'm going to fix these first. For the moment I'm ignoring MacOS which sometimes may behave differently, as you have seen.
I also may adapt some more tests this way and find more such issues. All this may take quite some time, so it may be a while before I go back to your issues.
Anyway, this has been inspired by your work with TSTL and is somewhat related to it.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Oct 5, 2017
@agroce
Copy link
Author

agroce commented Oct 9, 2017

PS, I'm using pyfakefs as a case study in a journal paper for December; I'm running the test harness on code mutants of fake_filessytem.py. Right now, we cover about 50% of the code with the harness that includes just os.open (vs. "Python" open), and of the mutants of that code, kill 64% reliably within 60 seconds. I'm investigating the mutants that are covered but not killed; many will simply be equivalent (reversing a list whose order doesn't matter) but some may show me some weakness in the test harness. I'll ping you once I am further along in this.

@mrbean-bremen
Copy link
Member

Sounds good! Didn't know that you are also into mutation testing, but could have guessed... Never had to do with that - it still sounds a little alien to me, but the concept is interesting.
On another note - I just set up Travis.CI to also test pyfakefs under MacOS (Python 2.7 and 3.6 - don't do all versions, as it takes quite some time due to resource problems). I will take the results I get for the tests there against the real file system as a standard for MacOS. There are a few tests that are not passing under real MacOS now, some are permission issues which I will ignore for now, others seem to be different exceptions, which I will look into in the near future.

@jmcgeheeiv
Copy link
Contributor

@agroce, just this morning I was promoting TSTL at my company.

Please make sure I a copy of your paper.

@agroce
Copy link
Author

agroce commented Oct 10, 2017

@jmcgeheeiv -- Thanks!

Will make sure to send you guys a copy once there's a draft, which might be a while (December deadline so other things in advance). It's a paper extending to testing the work in http://www.cs.cmu.edu/~agroce/ase15.pdf (which was on formal verification/bounded correctness proofs).

By the way, does it sound plausible that, right now when os.open and Python open can't be combined:

  • about 65% of the mutants that are of code we cover at all with the TSTL harness are detected by the os.open version of the harness

  • I'm not seeing yet any gain from the Python open in detection, though it runs a small amount of code (about 4% of the total code covered) that isn't run under the other setup

  • We only cover about half the mutants, but I know there are coverage gaps in the current testing, I just want to make sure there aren't also serious "bug detection" gaps

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 10, 2017

By the way, does it sound plausible that, right now when os.open and Python open can't be combined:

They can't be combined because of too many errors there?

about 65% of the mutants that are of code we cover at all with the TSTL harness are detected by the os.open version of the harness

I'm not sure I understand that. You have 2 versions of the harness, one for os.open, one for open? What code it is you are mutating? Sorry, I haven't used mutation testing (only read about it), so maybe I'm just lacking the understanding here.

@mrbean-bremen
Copy link
Member

Oh, and I just had a quick look at the paper you mentioned - I think I will check it out in the evening, this may answer some of my questions...

@agroce
Copy link
Author

agroce commented Oct 10, 2017

Alas, it won't really answer them that well. I'll expand on my sort of cryptic question.

What I'm mutating is the file fake_filesystem.py. I used muupi (https://github.com/apepkuss/muupi -- code by a former MS student of mine) to generate 800 or so variants, all of which are fake_filesystem.py + some small change. See https://github.com/agroce/cbmcmutate/tree/master/journal/pyfakefs_mutants for the files.

Probably some of these don't change semantics in a way that should be detected (e.g. they just reverse a list whose order is irrelevant). Many do something harmful. I'm trying to see which ones the TSTL harness can detect within 60 seconds, fairly reliably.

There is basically just one test harness, fakeosmodule.tstl (you can see the version I'm using here: https://github.com/agroce/cbmcmutate/blob/master/journal/fakeosmodule.tstl) but I can run it either:

  • with opener := newFakeOpener(~) commented out
  • or with all of the various ~.open calls commented out

So I only get one kind of file open in any test. The first version of the test harness seems to catch most (so far, all) of the mutants (changes to fake_filesystem.py) that I can detect. Which is about 64% of the ones whose code it even covers at all. Switching to using "Python open" so far adds no new problems detected, but it has a ways to go to check all the mutants.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 10, 2017

Ah, ok, that clarifies it a bit! I'm a little surprised that os.open test covers that much, though that maybe because both fake os.open and open rely on the same code - they basically differ only in buffering and some error handling (after translating modes).
What would be interesting to see (for me at least) is if all these mutations are also caught by our own tests - maybe I will make a quick test when I find the time.
What I don't understand is how you differentiate "benign" and "malignant" mutations between the ones the harness doesn't catch - do you check this manually?

@agroce
Copy link
Author

agroce commented Oct 10, 2017

Yes, unfortunately. In some language an optimizing compiler can detect some, but in Python it's manual work. This paper is partly about some ways of (maybe) making that a little less painful, if you're really interested in getting solid testing.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 10, 2017

Ah ok, that sounds interesting!

@agroce
Copy link
Author

agroce commented Oct 10, 2017

Basic idea is to use TSTL (or another tool) to generate a single test that:

  • covers the mutated code
  • covers as much other code as possible
  • and, of course, doesn't fail

The hope is that figuring out why that test passes (when the mutated code presumably interacts with as much as possible) might help.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 10, 2017

Nice - that means bringing together random testing and mutation testing, so that one verifies the other.

@agroce
Copy link
Author

agroce commented Oct 10, 2017

In the end, mutation testing is only useful when you combine it with something else (which is why mutation testing maybe wasn't the best name choice...)

I've got a file listing all the mutations of fake_filesystem.py that I covered, but didn't detect.

Some categories of missed mutants seem obvious:

  • st_atime, st_ctime, st_mtime modifications
  • ChangeDiskUsage modifications
  • epoch modifications
  • st_nlink modifications (was this ever fixed so we can test it?

Did we decide not to test st_mode for some reason? It's commented out.

Throwing out mutants that aren't killed, whose diff contains anything in: ["st_atime","st_ctime","st_mtime","ChangeDiskUsage","epoch","st_nlink","reversed"]
(I hand examined the "reversed" ones that are missed, and all are things like reversing order of going through mount points), we get to a 69% kill rate over covered code, and 128 mutants to examine by hand. I already see a few omissions from testing that can be fixed and will likely kill more mutants:

  • I never generate paths with components including upper/lower mixes, it's all lowercase, so self.is_case_sensitive can be modified without any changes
  • I never generate paths with a separator at the end

@agroce
Copy link
Author

agroce commented Oct 10, 2017

Adding case sensitivity seems to break things. Sending bug report. I'm on latest OS X, so both it and the fake file system (due to the platform check in init) should be non case sensitive.

@agroce
Copy link
Author

agroce commented Oct 10, 2017

Added a couple of bugs (or at least look like bugs) based on the mutant results. Symlink, no surprise.

Should we be fine with ignoring:

  • _last_ino changes

  • _last_dev changes

  • usage_change stuff

  • link_depth

if there are any of these that should break things if changed, or we should test, let me know

@agroce
Copy link
Author

agroce commented Oct 10, 2017

You can look at the github mutants directory, file interestingMutants.txt to see some not killed mutants. Diff is with respect to original.py which isn't quite your current version.

@mrbean-bremen
Copy link
Member

st_nlink modifications (was this ever fixed so we can test it?

Shall be fixed (see #255)

Should we be fine with ignoring:
_last_ino changes
_last_dev changes
usage_change stuff
link_depth

Mostly, I think. Incorrect _last_ino and _last_dev may lead to duplicate st_ino and st_dev, but st_ino is not used internally, and st_dev only for different mount points, that are not tested.
usage_change etc. is related to disk space, shall not be a problem, link_depth is used check for symlink loops, I don't think these are tested.

I probably will have a look at your results sometime later this week - thanks for the information!

@agroce
Copy link
Author

agroce commented Oct 11, 2017

Great. Just checked, and st_mode still mismatches on OS X, so I'll also consider that safe to ignore. Just submitted another case sensitivity related bug, discovered via the unkilled mutants telling me we weren't checking that code.

@mrbean-bremen
Copy link
Member

The st_mode stuff still seems to have some bugs - I will check this some time later.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Jan 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants