17:01:53 #startmeeting Ansible Testing Working Group 17:01:53 Meeting started Thu Jan 4 17:01:53 2018 UTC. The chair is mattclay. Information about MeetBot at http://wiki.debian.org/MeetBot. 17:01:53 Useful Commands: #action #agreed #halp #info #idea #link #topic. 17:01:53 The meeting name has been set to 'ansible_testing_working_group' 17:01:58 #chair gundalow 17:01:58 Current chairs: gundalow mattclay 17:02:14 hello 17:02:39 #chair sivel 17:02:39 Current chairs: gundalow mattclay sivel 17:02:46 #info Agenda: https://github.com/ansible/community/issues/248 17:03:58 * mattclay checks the agenda to see where we left off 17:05:37 If I'm remembering correctly, I think we've already covered everything on the agenda. 17:06:22 #topic Open Floor 17:06:32 * gundalow waves 17:07:45 gundalow, sivel: Do either of you have any updates or anything to discuss? 17:07:54 I have nothing 17:08:15 i have some tests related PR waiting for reviews if you want :) 17:08:22 #chair Pilou 17:08:22 Current chairs: Pilou gundalow mattclay sivel 17:08:58 #33766 (galaxy unit tests: add missing arg) and #33968 (control machine and managed host can be distinct) 17:09:23 I have two things 1) Can we detect bad usage of {} 2) I'm wondering once 2.5 is done to extend validate-modules to check more of DOCUMENATION vs argspec 17:11:45 By bad usage, I asume you are talking about '{}'.format(foo) ? 17:12:27 We might be able to put something together, although not sure how reliable it would be. I don't know of anything that does that check 17:13:20 We can probably add that as a custom pylint test. 17:13:45 I've started work on adding custom pylint tests to CI but haven't finished. 17:14:33 The validate-modules code for argspec should be functional, it's just not enabled to run by default 17:14:49 gundalow: past what it already does, are there anything it should be doing? 17:16:56 gundalow: I think we can but it will take a bit of coding and I haven't had time to prioritise it. 17:17:28 Pilou: Merged #33766 (galaxy unit tests: add missing arg) 17:17:45 I think you want to find string literals i nthe code (perhaps ast or perhaps jsut a regex) and then look for '{}'. If there are no false positives in our code base, then you're done. 17:17:52 If there are then you'll have to get trickier 17:18:11 mattclay: thanks! 17:21:13 sivel: Is it disabled by default for performance reasons? 17:22:12 mattclay: yes, it is very slow. It does some analysis, to find missing imports, magicmocks all the missing imports, imports the module, mocks AnsibleModule, runs `main()`, pulls out the argument_spec 17:22:26 well, maybe not too slow considering some of our other code smell tests and such 17:22:50 I honestly haven't run it in a while 17:24:43 sivel: Is the import mocking used to handle cases where imports from outside the standard library are imported without catching ImportError? 17:25:31 mattclay: yes, basically. In some cases even when wrapped in try/except with HAS_WHATEVER, we still need it to run 17:25:56 sivel: Why is that? 17:26:29 When I wrote it originally, there were some modules that used info from the imports in the arg spec 17:26:34 that may be different now 17:26:47 That should no longer be the case now that we have the import sanity test. 17:26:52 * mattclay checks 17:27:14 Ah, we still have 32 skip entries for the import test -- so we're not quite there yet. 17:27:42 sivel: How much of the slow performance is due to the import analysis and mocking vs the rest of the test? 17:27:42 someone would have to do some analysis on all of the imports that are mocked when doing that test 17:27:52 mattclay: I can't say right now 17:28:11 my thoughts are that the majority of the slowness is import mocks 17:28:36 I think it's definitely something we should look at doing if we can get the performance reasonable, which is sounds like we can if we can do away with the import mocks. 17:28:41 https://github.com/ansible/ansible/blob/devel/test/sanity/validate-modules/module_args.py#L43 17:29:25 Probably a first step will be to fix the remaining import issues: https://github.com/ansible/ansible/blob/devel/test/sanity/import/skip.txt 17:29:40 That way we shouldn't need to mock imports. 17:29:56 related to remaining import issue: #34076 :) 17:30:33 mattclay: I'll look at sticking `q` in the code or something, and log all of the modules it mocks 17:30:56 I've had it running for like 10 minutes now on all modules, so it 17:31:00 's a bit slow :) 17:32:11 (back) 17:32:39 Yes I was talking about '{}'.format(foo), It's something that's easily missed in reviews 17:32:55 Pilou: Merged 17:33:12 gundalow: I'll play with a quick regex and see what I can come up with 17:34:01 my current regex caught all 14 lines in that conjur_variable.py PR 17:34:20 it seems a custom pylint could be used too (https://github.com/PyCQA/pylint/blob/df53e1eb10c72c77036ef22e59e1b81f6b96582b/pylint/checkers/strings.py#L148) 17:34:41 yeah, we were talking about using a custom pylint plugin 17:35:26 It sound like I need to finish adding support for custom pylint plugins to ansible-test so we can start adding some for cases like this. 17:35:51 * mattclay looks for old branch 17:37:00 looks like W1305 already supports finding mixed indexed and non indexed format 17:37:11 could likely use part of that code 17:37:54 I found my branch for adding pylint plugin support (plus a few other pylint related changes). I'll look after this meeting to see what's left for that to be ready to use. 17:39:08 I think I put it on hold before due to pylint hanging -- but that has gotten better with newer pylint versions and other changes made to how we execute pylint since I last worked on plugin support. 17:49:00 alright, so fwiw, don't try to use `q` in that module_args.py code, seems to cause some really weird problems :) 17:52:52 Pilou: You've been doing a lot of import cleanup. Are you planning on working on any more? 17:54:06 mattclay: looks like even just checking `uri.py` causes it to mock 85 modules. Some of which are stdlib, so not sure why modulefinder lists them as badmodules 17:55:00 sivel: Does it work for `uri.py` if you disable the mocking? 17:55:18 I expect it should, since that module isn't on the skip list for the import test. 17:56:42 mattclay: yep 17:57:09 mattclay: interesting, it does seem to run fine 17:58:19 mattclay: even `hostname` which is on the skiplist seems to work 17:59:39 not all of them work though that are on the skip list 17:59:44 sivel: Probably because you have the deps installed that it needs. 18:00:08 would be odd to have the deps that yum needs on my mac, but not sure what they are 18:00:12 sivel: The import test uses an empty venv, so it's more picky about imports. 18:01:46 sivel: I thought you tested `hostname`, not `yum`? 18:01:56 I just tested all of them in skip.txt 18:02:17 only 3 failed for me 18:02:55 Well at least for `hostname` the import error only happens on Python 3.7. I'm checking the others now. 18:04:38 azure_rm_dnsrecordset.py dimensiondata_network.py gcdns_zone.py were the 3 that failed for me 18:04:50 sivel: It looks like the skip list is out-of-date. I'll clean it up. Some of the other imports errors are one you'll likely have installed. 18:05:37 cool, I'll play around a little more with that code and try to see if there are any other issues 18:05:54 OK. Anything else before we end the meeting? 18:06:17 Nothing else from me 18:06:22 Thanks for runningit 18:09:03 #endmeeting