| *** efried has joined #openstack-powervm | 13:10 | |
| edmondsw | efried I'm struggling to figure out how to mock the name attribute on a NamedTemporaryFile mock... any idea how to do that? | 19:02 |
|---|---|---|
| efried | edmondsw: Have you tried mock.patch.object? | 19:06 |
| efried | That won't actually work unless you're inside the context manager. | 19:09 |
| edmondsw | I think I've thought of another way to skin it | 19:19 |
| efried | Seems like you'd want to mock most of the tempfile code out anyway. Don't really want test cases creating temp files if we can help it. | 19:20 |
| edmondsw | efried why not? | 19:33 |
| efried | I'm trying to verbalize why that makes me uncomfortable. | 19:34 |
| efried | I mean, I guess there's not really a great reason. It just seems... icky. | 19:34 |
| efried | Also avoidable | 19:34 |
| efried | If it weren't for the fact that you had to cover it with mock.ANY, I could probably be convinced to let it ride. But that's ickier. | 19:35 |
| efried | edmondsw: I'm answering your original question (how to mock NamedTemporaryFile - so *almost* your original question) in the patch. | 19:41 |
| edmondsw | efried thanks. I spent way too much time on that today | 19:54 |
| edmondsw | I finally figured out how to make sure that mock_ccdi and mock_getsize at least saw the same temp file name, but beyond that... | 19:54 |
| efried | edmondsw: Responded. lmk if neither of those things works for you. The indirection to get at a context manager is kind of weird. the_mock.return_value.__enter__.return_value. | 19:55 |
| efried | Cleaner / easier-to-read but more LOC is making a real context manager for your mock's side effect. | 19:55 |
| efried | edmondsw: We should also test this live to make sure the docs aren't lying about e.g. being able to reopen the file. I guess you and mdrabe were discussing that yesterday. | 19:57 |
| edmondsw | I scripted up something similar that I could run standalone, and it worked, but I didn't actually try this with DevStack or anything | 19:58 |
| edmondsw | http://paste.openstack.org/show/732292/ if you're curious | 19:59 |
| edmondsw | the original code, scripted similarly, would break after 1000-something iterations because of the fd limit | 20:00 |
| edmondsw | this got past taht | 20:00 |
| efried | oh, you reproduced the fd limit, nice. | 20:00 |
| edmondsw | the test script you see called by subprocess there just echos x to fh.name | 20:00 |
| efried | sure, whatever that does isn't important (as long as it doesn't also leave the file open or something). | 20:01 |
| edmondsw | it's just important that it could write to the file and then the next line could read what was written | 20:01 |
| efried | Oh, yeah for sure, I tested that out locally too :) | 20:01 |
| efried | you can also get around this with mkstemp by explicitly os.close()ing the fd. And then you've still got to delete the file afterwards. Assuming the reopen trick works, getting NamedTemporaryFile to do those things for you is nice. | 20:02 |
| efried | presumably they've figured out the exact right way to do the deletion and stuff. | 20:02 |
| edmondsw | yep... first thing I did was just add os.close(fd) to the original code and verified that worked. Then worked on switching to NamedTemporaryFile | 20:06 |
| edmondsw | I'd wanted to do that originally, but didn't want to take the extra time involved... but when you suggested it, I said screw it, it's interesting, I'll do it | 20:06 |
| edmondsw | it is definitely the better way | 20:07 |
| edmondsw | efried ok, your method of mocking NamedTemporaryFile worked, so new patch is up for review | 20:07 |
| edmondsw | tx... and yuck, that is nasty | 20:07 |
| edmondsw | I hate mock | 20:07 |
| efried | edmondsw: Do we use config drive in our CI? | 20:10 |
| edmondsw | should be, yes | 20:10 |
| efried | k, then maybe we don't need to wait for live test? | 20:10 |
| edmondsw | yeah... I've been thinking that... but I'm unsure enough that CI would catch issues to want to do live testing | 20:11 |
| edmondsw | hence my query to mdrabe yesterday | 20:11 |
| edmondsw | I can try to test it myself, but I've already spent time on this that I can ill afford right now | 20:13 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!