Tuesday, 2018-10-16

*** efried has joined #openstack-powervm13:10
edmondswefried I'm struggling to figure out how to mock the name attribute on a NamedTemporaryFile mock... any idea how to do that?19:02
efriededmondsw: Have you tried mock.patch.object?19:06
efriedThat won't actually work unless you're inside the context manager.19:09
edmondswI think I've thought of another way to skin it19:19
efriedSeems 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
edmondswefried why not?19:33
efriedI'm trying to verbalize why that makes me uncomfortable.19:34
efriedI mean, I guess there's not really a great reason. It just seems... icky.19:34
efriedAlso avoidable19:34
efriedIf 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
efriededmondsw: I'm answering your original question (how to mock NamedTemporaryFile - so *almost* your original question) in the patch.19:41
edmondswefried thanks. I spent way too much time on that today19:54
edmondswI 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
efriededmondsw: 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
efriedCleaner / easier-to-read but more LOC is making a real context manager for your mock's side effect.19:55
efriededmondsw: 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
edmondswI scripted up something similar that I could run standalone, and it worked, but I didn't actually try this with DevStack or anything19:58
edmondswhttp://paste.openstack.org/show/732292/ if you're curious19:59
edmondswthe original code, scripted similarly, would break after 1000-something iterations because of the fd limit20:00
edmondswthis got past taht20:00
efriedoh, you reproduced the fd limit, nice.20:00
edmondswthe test script you see called by subprocess there just echos x to fh.name20:00
efriedsure, whatever that does isn't important (as long as it doesn't also leave the file open or something).20:01
edmondswit's just important that it could write to the file and then the next line could read what was written20:01
efriedOh, yeah for sure, I tested that out locally too :)20:01
efriedyou 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
efriedpresumably they've figured out the exact right way to do the deletion and stuff.20:02
edmondswyep... first thing I did was just add os.close(fd) to the original code and verified that worked. Then worked on switching to NamedTemporaryFile20:06
edmondswI'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 it20:06
edmondswit is definitely the better way20:07
edmondswefried ok, your method of mocking NamedTemporaryFile worked, so new patch is up for review20:07
edmondswtx... and yuck, that is nasty20:07
edmondswI hate mock20:07
efriededmondsw: Do we use config drive in our CI?20:10
edmondswshould be, yes20:10
efriedk, then maybe we don't need to wait for live test?20:10
edmondswyeah... I've been thinking that... but I'm unsure enough that CI would catch issues to want to do live testing20:11
edmondswhence my query to mdrabe yesterday20:11
edmondswI can try to test it myself, but I've already spent time on this that I can ill afford right now20:13

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!