17:02:34 #startmeeting Testing Working Group 17:02:34 Meeting started Thu Jul 13 17:02:34 2017 UTC. The chair is mattclay. Information about MeetBot at http://wiki.debian.org/MeetBot. 17:02:34 Useful Commands: #action #agreed #halp #info #idea #link #topic. 17:02:34 The meeting name has been set to 'testing_working_group' 17:02:52 #chair gundalow 17:02:52 Current chairs: gundalow mattclay 17:02:59 * mattclay waves 17:03:01 * Pilou waves 17:03:06 #chair Pilou 17:03:06 Current chairs: Pilou gundalow mattclay 17:03:46 * mattclay skims https://github.com/ansible/community/issues/114 for open items to discuss 17:04:23 * spredzy waves 17:04:35 #chair spredzy 17:04:35 Current chairs: Pilou gundalow mattclay spredzy 17:04:47 A few updates before we get started: 17:05:08 We have a new "import" sanity test in ansible-test / CI now. 17:05:13 #info :) 17:05:22 gundalow: Thanks 17:05:28 For all the people that look at the logs *cough* 17:06:02 #info We have a new "import" sanity test in ansible-test and CI now. It will detect imports of python modules not in the standard library in either modules or module_utils which are not protected by a try/except block. 17:06:29 #info We will be extending this test to cover plugins as well. 17:07:00 #info We will be dropping testing of Fedora 24 and 25 with python 2 on Shippable and adding Fedora 25 and 26 with python 3. 17:07:34 gundalow: Do you have anything? 17:08:49 nopt 17:08:54 * gundalow -> afk 17:09:06 #topic Replacing pep8 with pycodestyle (https://github.com/ansible/ansible/pull/25947) 17:09:24 This has been on the agenda for a while, but hasn't been discussed yet. 17:09:52 Pilou: This one's yours. :) 17:10:03 yep :) 17:11:07 The changes look good. I'm OK with keeping the test named pep8 even though the tool running the test is pycodestyle, especially since it's ultimately enforcing PEP 8 rules. 17:11:22 So +1 from me 17:11:40 Does anyone else have comments? 17:11:51 we might want to fix E741 17:12:00 which is "ambiguous variable name" 17:13:06 there isn't many occurrence of this error in the current code 17:13:23 That's the biggest change with moving to pycodestyle. Using pep8 has been stable since it's no longer updated. Using pycodestyle is going to give us a moving target as more rules are added. 17:13:36 Which will periodically break CI until we either fix or ignore the issues. 17:14:01 If it gets to be a problem we can switch to a whitelist of rules to enforce instead of a blacklist. 17:14:21 Pilou: Once it's merged you can submit a separate PR to fix all the E741 issues at once. 17:14:34 ok 17:14:42 How many are there anyways? 17:14:43 hum, the version of pycodestyle package should be enforced 17:15:04 For the migration to be seemless you can also make test fail only is they are more failures on current commit then previous 17:15:24 This way one doesn't get penalized by a -1 unless is code is doing something wrong 17:15:26 spredzy: We've got that covered already with our ignore list. 17:15:45 ack 17:16:07 Pilou: We typically only require specific version when they're either 1) broken or 2) older versions lack features we require 17:16:22 Shippable will always run with the latest version. 17:16:31 Users are free to use whatever they want locally. 17:16:38 but new version of pycodestyle could break the CI 17:16:42 Obviously that can lead to differences between local testing and CI. 17:17:09 Yes, which makes it obvious we have something to address. We can either decide to ignore or fix. 17:17:39 If it becomes a frequent occurrence we'll change our approach. 17:17:58 ok 17:18:22 If we pin the version then new issues that could be detected will go unnoticed. 17:18:45 Until someone runs locally with a newer version and doesn't use our requirements file. 17:19:18 Are there any objections on merging this PR then? 17:20:05 nop 17:21:07 I'm re-running CI on the PR now. 17:21:15 #action mattclay to merge PR once CI passes 17:22:11 #topic Community Wiki (https://github.com/ansible/community/issues/114#issuecomment-310875236) 17:22:22 I don't believe we've discussed the community wiki here yet. 17:23:07 Does anyone have anything to add to the testing group page here? https://github.com/ansible/community/tree/master/group-testing 17:23:42 Or the wiki page here? https://github.com/ansible/community/wiki 17:24:11 Sorry, testing specific link for the wiki: https://github.com/ansible/community/wiki/Testing 17:24:17 back 17:24:41 * mattclay away for a minute 17:25:51 #info Testing Wiki exists at https://github.com/ansible/community/wiki/Testing 17:26:12 Thanks to those that have added stuff so far 17:26:37 * mattclay is back 17:26:39 Pilou: Do you have have edit permissions on the wiki? 17:27:08 if not, would you like them? 17:27:27 gundalow: yep, i have 17:27:33 Great 17:28:10 Speaking of things to put on the wiki. Does anyone want to volunteer to fix existing modules/module_utils which fail the new import sanity test? 17:30:19 mattclay: Where is the test defined, e.g. how can people see the todo list? 17:30:41 #info Existing files which don't pass the import sanity test: https://github.com/ansible/ansible/blob/devel/test/sanity/import/skip.txt 17:31:09 They're pretty easy to fix, but there's a lot of them. 17:31:21 * gundalow wonders if I should add that list to the wiki, feels like anyone that's comfortable with Python could chip away at the list 17:31:29 * mattclay nods 17:33:03 and it's: ansible-test sanity --test import 17:34:36 #topic Open Floor 17:35:06 One thing I forget in my earlier update. We now have our largest module with 100% code coverage: ping :) 17:35:07 https://codecov.io/gh/ansible/ansible/src/devel/lib/ansible/modules/system/ping.py 17:35:41 mattclay: for import/skip are we specifically lookin for module_utils, or any/all to be fixed? 17:35:47 This is a side-effect of the new import sanity test, as it covers the other half of the `if __name__ == '__main__'` branch not hit during normal module execution. 17:36:03 gundalow: module_utils and modules both 17:36:10 ack 17:36:30 Plugins too, once the test is updated to include them. 17:36:59 What's the point of `data: crash` in the ping module? 17:37:37 I believe it's there for testing when you want a module to raise an unhandled exception. 17:38:34 ah, OK 17:38:59 Now that's it possible for modules to reach 100% code coverage, I'm hoping we'll start seeing some slightly more complex modules hitting that. 17:39:16 https://github.com/ansible/community/wiki/Testing%3A-progress-tracker/_edit#fixing-broken-imports added 17:39:24 :) 17:39:38 about pycodestyle and E741, there are only 9 occurrences of this error 17:40:19 #25798 : a test_pull_requests & stale_ci labele PR, reviews are welcome 17:43:24 Pilou: Looks good. I've restarted CI on that one. 17:44:47 Does anyone have anything else to discuss? 17:44:49 about new 'import' test, i locally deleted 'lib/ansible/modules/cloud/amazon/cloudformation.py' from 'test/sanity/import/skip.txt' and 'ansible-test sanity --python 2.7 --tox --test import' fails with 17:44:51 'ERROR: lib/ansible/modules/cloud/amazon/cloudformation.py:259:0: ImportError: No module named utils.hashing' 17:45:08 is that a false positive ? 17:45:19 Pilou: No, it's importing something from outside module_utils. 17:45:45 Pilou: That code needs to be copied to module_utils or the module itself. 17:46:11 ok, thanks for the explanation :) 17:46:40 It probably works for most people because they run the module on their local host where ansible is installed. 17:46:49 If you ran the module on a remote host without ansible it would fail. 17:47:31 when the code is copied to module_utils, is there a licence issue ? 17:48:08 You'll need to look at the licenses in both places. 17:48:28 It might be better to copy the code to the module, since there's only one usage. 17:48:56 ok 17:49:38 when the code should be moved to module_utils, what is the workflow to follow ? asking all authors of the file is they are ok ? 17:50:08 abadger1999: ^ I believe you've had to deal with this before? 17:50:38 * abadger1999 looks 17:50:48 Pilou: yeah :-( that's the way to do it. 17:51:01 We can talk about alternatives if there's authors you can't find. 17:51:21 but that will involve real legal-knowledgable people 17:52:09 Pilou: email (with complete headers) is one way to get consent. And we have started to use acceptance on a github ticket for that purpose as well. 17:52:45 Pilou: start with the easy ones :) 17:53:01 :) 17:53:26 It's about reducing the list, doesn't matter what order that's done in 17:53:36 And thanks in advance for your help with it :) 17:53:50 Pilou: If you copy to the module it should be OK, they're both GPL 3. 17:54:54 can i interject a testing question? 17:55:02 jtanner: Sure 17:55:18 is there a way to tell ansible-test not to tear down the containers at the end of the test? 17:55:27 specifically for the vcsim container 17:55:46 jtanner: Not currently. 17:55:50 crud 17:55:58 i guess i'll just do it manually 17:56:02 first world problems 17:56:02 We have --remote-terminate for AWS, don't seem to have the same for Docker 17:57:31 OK, anything else? We're almost at the end of our scheduled meeting. 17:57:40 Nothing else from me 17:58:53 #endmeeting