| *** rbrady has quit IRC | 00:43 | |
| *** rbrady has joined #openstack-mistral | 00:45 | |
| *** zhurong has joined #openstack-mistral | 01:13 | |
| *** chlong__ has joined #openstack-mistral | 01:55 | |
| *** gongysh has joined #openstack-mistral | 02:05 | |
| *** zhurong has quit IRC | 02:15 | |
| *** zhurong has joined #openstack-mistral | 02:19 | |
| *** catintheroof has quit IRC | 02:21 | |
| *** catintheroof has joined #openstack-mistral | 02:23 | |
| *** catintheroof has quit IRC | 02:27 | |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Add hacking for code style checks https://review.openstack.org/435395 | 05:14 |
|---|---|---|
| *** sharatss has joined #openstack-mistral | 05:14 | |
| *** sharatss has quit IRC | 06:12 | |
| *** sharatss has joined #openstack-mistral | 06:16 | |
| *** sharat has joined #openstack-mistral | 06:19 | |
| *** sharatss has quit IRC | 06:32 | |
| *** sharatss has joined #openstack-mistral | 06:41 | |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Add hacking for code style checks https://review.openstack.org/435395 | 06:52 |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Added style enfore checks for assert statements https://review.openstack.org/431427 | 06:57 |
| *** sharat has quit IRC | 07:06 | |
| *** jaosorior has joined #openstack-mistral | 07:30 | |
| *** chlong__ has quit IRC | 07:36 | |
| d0ugal | Morning! | 07:48 |
| d0ugal | ddeja: Would you have time to review this patch series? https://review.openstack.org/#/c/440633/ (see related changed) | 07:49 |
| d0ugal | changes* | 07:49 |
| ddeja | d0ugal: I can't promise anything | 07:59 |
| d0ugal | ddeja: okay, no problem :) | 08:00 |
| * ddeja adds link to chrome hoping to have some time today :) | 08:00 | |
| d0ugal | ddeja: thanks very much! | 08:00 |
| d0ugal | maybe mgershen will have time to look at the others :) | 08:00 |
| openstackgerrit | Merged openstack/mistral-extra master: Set up mistral-extra with the ability to run unit tests https://review.openstack.org/437058 | 08:04 |
| d0ugal | Need a second review on https://review.openstack.org/#/c/440633 and https://review.openstack.org/#/c/440646 for the series to land :) | 08:16 |
| d0ugal | Three patches have +W but depend on those (becaues thrash|g0ne wanted faster tests!) | 08:17 |
| ddeja | d0ugal: done | 08:27 |
| d0ugal | ddeja: thanks! | 08:27 |
| * ddeja didn't notice at the first time that those 2 are short patches | 08:28 | |
| ddeja | d0ugal: both +2 +W | 08:28 |
| ddeja | ;) | 08:28 |
| d0ugal | \o/ | 08:28 |
| d0ugal | The test suite will be twice as fast, maybe more, for everyone soon! | 08:28 |
| openstackgerrit | Renat Akhmerov proposed openstack/mistral master: Rename package 'workbook' to 'lang' https://review.openstack.org/441093 | 08:42 |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Added style enfore checks for assert statements https://review.openstack.org/431427 | 08:44 |
| d0ugal | In https://review.openstack.org/#/c/437212/ the coverage got worse in files that were not even changed :/ | 08:45 |
| d0ugal | well, it seems one of them was changed but not the other three | 08:45 |
| d0ugal | rakhmerov: does the coverage report make sense to you? | 08:45 |
| rakhmerov | d0ugal: I asked Michal about an hour ago to review these two patches | 08:46 |
| d0ugal | rakhmerov: the two I asked about? Dawid already has :) | 08:47 |
| rakhmerov | looking at the report... | 08:47 |
| rakhmerov | ooh, good | 08:47 |
| d0ugal | Does the coverage report rebase the patch on master before comparing? | 08:48 |
| d0ugal | If it doesn't, then the logic is broken | 08:48 |
| rakhmerov | not sure actually | 08:48 |
| rakhmerov | I think it doesn't | 08:48 |
| rakhmerov | so you think it's ok to approve it? | 08:49 |
| d0ugal | I think so | 08:49 |
| rakhmerov | I'm ok with this | 08:49 |
| rakhmerov | just wanted kong to make sure that this is ok | 08:49 |
| d0ugal | It reduces coverage in one file by 1 line, but I can't even tell what line that is | 08:49 |
| d0ugal | Yeah, it makes sense. | 08:49 |
| rakhmerov | ok, let's approve it | 08:49 |
| rakhmerov | I like the patch itself | 08:49 |
| d0ugal | I am just trying to understand the coverage check :) | 08:50 |
| d0ugal | I didn't actually review it because I seen your comment, I w8ll not | 08:50 |
| rakhmerov | sharatss should be able to help ) | 08:50 |
| d0ugal | I will now | 08:50 |
| rakhmerov | ok | 08:50 |
| *** jpich has joined #openstack-mistral | 08:50 | |
| rakhmerov | d0ugal: yeah, it's weird. It shows that in default_executor coverage got worse | 08:53 |
| rakhmerov | although it wasn't touched at all | 08:53 |
| rakhmerov | well, it might make sense actually | 08:53 |
| d0ugal | and I can't even guess which line in api.py isn't covered | 08:54 |
| d0ugal | because there are no new branches etc. | 08:54 |
| rakhmerov | think about this situation: I changed something in a different file (e.g. a test) and because of this some lines in the executor never ran | 08:54 |
| rakhmerov | in api.py I think I found it.. | 08:55 |
| rakhmerov | sec | 08:55 |
| rakhmerov | http://logs.openstack.org/12/437212/3/check/mistral-coverage-ubuntu-xenial-nv/810b94e/cover/mistral_db_v2_sqlalchemy_api_py.html | 08:56 |
| rakhmerov | there's a lot of lines that are not covered in api | 08:56 |
| rakhmerov | red ones | 08:56 |
| d0ugal | Yup | 08:56 |
| d0ugal | But which one was introduced by this patch? | 08:56 |
| rakhmerov | hard to tell, the relationship may be indirect | 08:57 |
| rakhmerov | see my previous note on executor | 08:57 |
| d0ugal | Indeed | 08:57 |
| d0ugal | but I am suspicious of the coverage report :) | 08:57 |
| rakhmerov | :) | 08:57 |
| rakhmerov | what makes me surprised is that look at api.py again | 08:57 |
| d0ugal | If the parent of the patch isn't master, then the report is likely to be wrong | 08:57 |
| rakhmerov | methods get_workflow_definition_by_id and get_action_definition_by_id are not covered | 08:58 |
| rakhmerov | how come?? | 08:58 |
| rakhmerov | basic stuff that should be used in lots of places | 08:58 |
| rakhmerov | ooh, I see. Mostly workflows and actions are referred to by name | 08:59 |
| d0ugal | rakhmerov: so the parent of https://review.openstack.org/#/c/437212/ is https://github.com/openstack/mistral/commit/b6b40cbbc1d64a501f29870490a1b92724dfebc1 | 08:59 |
| d0ugal | that means we are comparing the coverage of that parent + this patch against master | 09:00 |
| *** shardy has joined #openstack-mistral | 09:00 | |
| d0ugal | There have been 16 commits to master since that patch | 09:00 |
| d0ugal | (including merge commits) | 09:00 |
| rakhmerov | :)) | 09:00 |
| rakhmerov | wait a sec... | 09:00 |
| rakhmerov | I wanna check this | 09:00 |
| rakhmerov | I wanna look at cover.sh | 09:01 |
| d0ugal | so they could have changed the coverage in master by a large number of lines (we allow up to 4 regressions per commit) | 09:01 |
| d0ugal | so I think we are comparing the wrong things. | 09:01 |
| d0ugal | to summarise - if the 16 commits after that patch improved coverage in master, this patch might seem to make it worse as it is missing those patches - so it needs to be rebased on master to include them. | 09:02 |
| d0ugal | (or the commits after it might make master worse, each doing it by one or two lines, then this patch would make it look much better but actually it isn't) | 09:03 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Fix update workflow by admin https://review.openstack.org/437212 | 09:04 |
| d0ugal | to test this I have rebased the patch, we should get a different result now. | 09:04 |
| rakhmerov | d0ugal: yeah, I think you're right | 09:04 |
| d0ugal | we shall see when the coverage check completes on my the rebased patch :) | 09:05 |
| d0ugal | I think this is also why people have been seeing different results between different patch sets because they are sometimes rebasing | 09:06 |
| d0ugal | (I noticed some comments before) | 09:06 |
| d0ugal | oh, but it doesn't actually checkout master | 09:07 |
| d0ugal | it uses HEAD^, I thought that would be correct. So now I am confused! | 09:07 |
| d0ugal | Why is nothing simple :P | 09:07 |
| rakhmerov | yeah :) | 09:09 |
| rakhmerov | I just saw HEAD^ too | 09:10 |
| rakhmerov | hm... | 09:10 |
| rakhmerov | interesting | 09:10 |
| d0ugal | I'll test the script locally and see if I can make sense of it | 09:10 |
| rakhmerov | btw, the commend in cover.sh is then wrong | 09:11 |
| rakhmerov | it says "checkout master" | 09:11 |
| rakhmerov | but it's not master | 09:11 |
| d0ugal | yeah, and it echo's master too | 09:11 |
| d0ugal | on line 46 | 09:12 |
| rakhmerov | yep | 09:12 |
| d0ugal | that is why I came to the conculsion that I did :) I'll update the code to correct that at least | 09:12 |
| rakhmerov | yes, pls | 09:12 |
| d0ugal | Outputting the sha of the commit we are comparing with would be useful I think | 09:12 |
| rakhmerov | yp | 09:13 |
| *** d0ugal has quit IRC | 09:14 | |
| jaosorior | How can I get more info on the failure of an execution? | 09:16 |
| sharatss | rakhmerov: sorry for joining late | 09:23 |
| sharatss | the cover.sh compares the whole repo with the patch | 09:23 |
| sharatss | so the failure need not be necessarily caused due to the patch committed. It includes all the poorly covered files | 09:24 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Add `coverage erase` to the cover report https://review.openstack.org/441831 | 09:26 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Don't use 'master' as that isn't always true https://review.openstack.org/441832 | 09:26 |
| *** d0ugal has joined #openstack-mistral | 09:26 | |
| * d0ugal had a connection issue | 09:26 | |
| sharatss | d0ugal: how about removing the cover.sh script and generating the coverage directly from tox | 09:28 |
| sharatss | ?? | 09:28 |
| rakhmerov | sharatss: we were discussing a different issue | 09:28 |
| rakhmerov | the report itself is ok | 09:29 |
| rakhmerov | but this new feature that shows if coverage became worse doesn't seem to work transparently | 09:29 |
| rakhmerov | jaosorior: hi, what do you mean? | 09:29 |
| rakhmerov | provide more context pls.. | 09:30 |
| d0ugal | jaosorior: I think I missed the question here - my connection dropped briefly. | 09:30 |
| jaosorior | rakhmerov: sorry about that, found out how. Just a matter of doing execution-output-get | 09:30 |
| rakhmerov | ooh, yes | 09:31 |
| rakhmerov | ok | 09:31 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: TESTING Fix update workflow by admin https://review.openstack.org/441840 | 09:31 |
| sharatss | rakhmerov: ohh.. ok got it.. | 09:32 |
| d0ugal | rakhmerov: so I copied kongs change to a new review and rebased it on my changes to see if it helps at all. https://review.openstack.org/#/c/441840/ | 09:35 |
| sharatss | rakhmerov: need to erase the previous report before starting to make the coverage report | 09:35 |
| d0ugal | sharatss: I just added a patch for that :) | 09:35 |
| d0ugal | https://review.openstack.org/#/c/441831 | 09:35 |
| d0ugal | waiting to see the output | 09:35 |
| sharatss | d0ugal: yea.. that is it!! | 09:36 |
| sharatss | d0ugal: can we just move the whole coverage thing to tox.ini to make it less confusing? | 09:37 |
| sharatss | d0ugal: at least we have one less script to maintain | 09:37 |
| d0ugal | sharatss: yeah, that would be good - why was it done in a script originally? | 09:38 |
| d0ugal | I would never choose a bash script because I don't really understand them :-D | 09:38 |
| d0ugal | rakhmerov: btw https://review.openstack.org/#/c/437212/ failed with 4 lines missing coverage again - but two of the files were different! | 09:38 |
| rakhmerov | :) | 09:39 |
| d0ugal | http://logs.openstack.org/12/437212/4/check/mistral-coverage-ubuntu-xenial-nv/c361527/console.html#_2017-03-06_09_19_16_694739 | 09:39 |
| rakhmerov | d0ugal, sharatss: what do you mean by moving it to tox.ini? | 09:39 |
| rakhmerov | not sure I understand | 09:39 |
| sharatss | under [testenv:cover] we will have python setup.py testr --slowest --testr-args='{posargs}' which works similarly | 09:40 |
| sharatss | we can add coverage erase in the same field which is similar to https://review.openstack.org/#/c/441831 | 09:41 |
| d0ugal | yeah, other projects do this. For example: https://github.com/openstack/python-tripleoclient/blob/master/tox.ini#L35 | 09:41 |
| sharatss | rakhmerov: if you want a clear idea, i will just test commit the change | 09:43 |
| rakhmerov | that's ok, I got it | 09:43 |
| sharatss | should check if that thing helps | 09:44 |
| d0ugal | sharatss: I think it is a good idea | 09:46 |
| d0ugal | but I think something is wrong in the logic, so I am going to continue trying to find out what :) | 09:46 |
| sharatss | d0ugal: yes sure :) | 09:47 |
| d0ugal | rakhmerov, sharatss - adding erase did make a big difference. | 09:51 |
| d0ugal | Compare: | 09:51 |
| d0ugal | http://logs.openstack.org/40/441840/1/check/mistral-coverage-ubuntu-xenial-nv/6b2741f/console.html#_2017-03-06_09_48_56_004427 | 09:51 |
| d0ugal | ad | 09:51 |
| d0ugal | and http://logs.openstack.org/12/437212/4/check/mistral-coverage-ubuntu-xenial-nv/c361527/console.html#_2017-03-06_09_19_16_694739 | 09:51 |
| d0ugal | They are the same patch (I just copied it) but the first link is based on my two changes to the coverage report | 09:51 |
| d0ugal | The script is also correctly using the parent of the patch - so that is fine. | 09:52 |
| d0ugal | I think erase is all we needed really but showing the parent commit sha is useful IMO - it makes it a bit clearer. | 09:52 |
| rakhmerov | ok, great ) | 09:53 |
| d0ugal | so now I just need to find another core :) | 09:53 |
| openstackgerrit | Renat Akhmerov proposed openstack/mistral master: WF global ctx: add "publish-global" into direct workflow task spec https://review.openstack.org/441851 | 09:54 |
| d0ugal | sharatss: so I think we should merge my improvements, then we can refactor it and move it into tox - I'll let you do that if you want :) | 09:54 |
| rakhmerov | ok, np | 09:55 |
| sharatss | d0ugal: but in tox.ini we need not do any changes related to previous sha i guess. we need to just add the coverage and erase coverage | 09:57 |
| openstackgerrit | Merged openstack/mistral master: Reduce the number of with-items and retried in the concurrency test https://review.openstack.org/440633 | 09:58 |
| openstackgerrit | Merged openstack/mistral master: Verify the retry policy when passed in via variables https://review.openstack.org/440646 | 09:58 |
| openstackgerrit | Merged openstack/mistral master: Every unit test creates and registers every OpenStack action https://review.openstack.org/441038 | 09:58 |
| openstackgerrit | Merged openstack/mistral master: Use utcnow() in expired executions policy test https://review.openstack.org/441082 | 09:58 |
| openstackgerrit | Merged openstack/mistral master: Fix get_next_execution_time https://review.openstack.org/441083 | 09:59 |
| d0ugal | sharatss: I'm not sure I fully understand. So we would still have the script for the coverage gate? | 09:59 |
| d0ugal | \o/ those merges! | 09:59 |
| sharatss | d0ugal: if we moved it into tox then we can eliminate the script | 09:59 |
| sharatss | or if the script is working fine now, then it might be safe to keep it\ | 10:00 |
| d0ugal | sharatss: but how would we compare with the previous sha etc. to determine the change in coverage? | 10:00 |
| d0ugal | or are you suggesting we don't do that anymore? | 10:00 |
| d0ugal | I think the script is okay now, it has been confusing but I think that was just because we were missing the erase step. | 10:01 |
| d0ugal | so maybe we can just keep it and see how it is this week. | 10:01 |
| sharatss | d0ugal: thats a better and safe thing to do | 10:03 |
| kong | d0ugal: thanks for the investigation you did just now :) for better coverage comparison | 10:05 |
| d0ugal | kong: no problem - sorry for the extra gerrit notifications I caused! | 10:06 |
| kong | d0ugal: you did a right thing, man | 10:06 |
| kong | no need to apologize | 10:06 |
| d0ugal | kong: can you review https://review.openstack.org/#/c/441831/ and https://review.openstack.org/#/c/441832/ if you have a moment? | 10:07 |
| d0ugal | That is the outcome of my investigation. | 10:07 |
| kong | d0ugal: sure, i'm looking at the coverage erase one | 10:07 |
| kong | the other one is good to me | 10:08 |
| d0ugal | great :) | 10:08 |
| d0ugal | I should maybe have explained more in the coverage erase patch. | 10:08 |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Added style enfore checks for assert statements https://review.openstack.org/431427 | 10:08 |
| kong | d0ugal: both are pretty straightforward, will approve after jenkins pass | 10:08 |
| d0ugal | thanks | 10:09 |
| kong | np :0 | 10:09 |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Add hacking for code style checks https://review.openstack.org/435395 | 10:13 |
| *** openstackgerrit has quit IRC | 10:33 | |
| *** openstackgerrit has joined #openstack-mistral | 10:33 | |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Improve the CONTRIBUTING.rst https://review.openstack.org/424058 | 10:33 |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: Changed the README.rst and added debug guide https://review.openstack.org/420687 | 10:51 |
| openstackgerrit | Sharat Sharma proposed openstack/python-mistralclient master: Add script for unit test coverage job https://review.openstack.org/417902 | 10:54 |
| openstackgerrit | Merged openstack/mistral-specs master: Add a policy section to the Mistral specs https://review.openstack.org/440362 | 10:59 |
| kong | d0ugal: coverage test was failed for https://review.openstack.org/#/c/441832/, is it expected? | 11:03 |
| d0ugal | kong: haha, no, that is curious | 11:04 |
| d0ugal | kong: I'll look into it, but that is very strange. There must be another problem with it still. | 11:05 |
| d0ugal | also, I made a typo in that commit | 11:05 |
| kong | d0ugal: thanks, i have to go to bed, will take a look at it again tomorrow :) | 11:06 |
| d0ugal | kong: okay, thanks - good night! | 11:06 |
| kong | & | 11:06 |
| * d0ugal feels confused saying that at 11am | 11:07 | |
| *** fultonj has joined #openstack-mistral | 11:08 | |
| d0ugal | mgershen: I don't know :) | 11:17 |
| mgershen | d0ugal: the more I look at coverage tests the more confused I get | 11:17 |
| d0ugal | mgershen: haha, I agree | 11:17 |
| mgershen | d0ugal: I'm somewhere between crying and laughing | 11:18 |
| openstackgerrit | Merged openstack/mistral master: Fix update workflow by admin https://review.openstack.org/437212 | 11:20 |
| *** gongysh has quit IRC | 11:21 | |
| mgershen | d0ugal: Is there away to publish the previous commit cover result? | 11:21 |
| d0ugal | mgershen: Yeah, there should be - I'll add that. | 11:22 |
| mgershen | d0ugal: for example here is the cover of your PS http://logs.openstack.org/32/441832/1/check/mistral-coverage-ubuntu-xenial-nv/ab6f7b9/cover/ | 11:22 |
| d0ugal | mgershen: yeah, I mean, that patch shows the report is completely wrong as it doesn't touch any Python code :( | 11:23 |
| d0ugal | I'm just updating another patch, then I'll go back to this | 11:23 |
| mgershen | d0ugal: cool, maybe it will help us understand where did 3 statements go missing from mistral/engine/default_executor.py | 11:24 |
| *** shardy has quit IRC | 11:26 | |
| *** shardy has joined #openstack-mistral | 11:26 | |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Add documentation for the engine commands https://review.openstack.org/435438 | 11:29 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Add documentation for the engine commands https://review.openstack.org/435438 | 11:30 |
| d0ugal | mgershen: I am trying to find out how to change the directory where the report is generated | 11:41 |
| d0ugal | any idea how to do that in coverage? | 11:41 |
| d0ugal | it seems that the ansible playbook uploads the "cover" directory - so I just need to output to "cover/patch-patch" and "cover/previous-report" - or something like that. | 11:42 |
| *** fultonj has quit IRC | 11:45 | |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Store the previous and current coverage reports https://review.openstack.org/441902 | 11:50 |
| * d0ugal sits and watches zuul | 11:51 | |
| *** zhurong has quit IRC | 11:58 | |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Store the previous and current coverage reports https://review.openstack.org/441902 | 12:09 |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: [doc] Changed the output fields in quickstart guide https://review.openstack.org/441916 | 12:21 |
| *** shardy is now known as shardy_lunch | 12:21 | |
| openstackgerrit | Merged openstack/mistral master: Add `coverage erase` to the cover report https://review.openstack.org/441831 | 12:33 |
| *** jamielennox is now known as jamielennox|away | 12:50 | |
| *** dprince has joined #openstack-mistral | 12:50 | |
| *** thrash|g0ne is now known as thrash | 12:55 | |
| *** shardy_lunch is now known as shardy | 12:56 | |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Don't use 'master' as that isn't always true https://review.openstack.org/441832 | 13:00 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Include the missing lines in the coverage report https://review.openstack.org/441902 | 13:00 |
| *** jkilpatr has joined #openstack-mistral | 13:12 | |
| d0ugal | oh man, this coverage stuff is so complicated. | 13:14 |
| thrash | :P | 13:15 |
| d0ugal | This patch, aparently changes the coverage result quite a bit. https://review.openstack.org/#/c/441902 | 13:15 |
| d0ugal | http://logs.openstack.org/02/441902/3/check/mistral-coverage-ubuntu-xenial-nv/b6a1bd3/console.html#_2017-03-06_13_09_29_813110 | 13:15 |
| d0ugal | Some files have the same coverage (by count of missing lines etc) but different lines are missing or not | 13:16 |
| d0ugal | Do we have something random in the tests? | 13:16 |
| openstackgerrit | Brad P. Crochet proposed openstack/python-mistralclient master: Add --utc flag to cron trigger create https://review.openstack.org/441350 | 13:17 |
| d0ugal | I think maybe we should just make it simpler and only compare the coverage percentage. | 13:17 |
| openstackgerrit | Sharat Sharma proposed openstack/mistral master: DNM - Test for coverage https://review.openstack.org/441940 | 13:20 |
| d0ugal | sharatss: I think we need to rethink how we check coverage | 13:24 |
| sharatss | d0ugal: i was verifying with the murano guys, they also pretty much use the same script | 13:25 |
| sharatss | d0ugal: i am unable to figure out why we are having problems | 13:25 |
| sharatss | :( | 13:25 |
| d0ugal | http://logs.openstack.org/02/441902/3/check/mistral-coverage-ubuntu-xenial-nv/b6a1bd3/console.html#_2017-03-06_13_09_29_813110 | 13:25 |
| d0ugal | sharatss: I added the missed lines to the output in that patch | 13:26 |
| d0ugal | the results just seem random | 13:26 |
| sharatss | d0ugal: is there something wrong with the script? | 13:27 |
| sharatss | shall we try in tox and check what results we get? | 13:27 |
| d0ugal | sharatss: Not the script exactly | 13:27 |
| d0ugal | the problem is more that the coverage report is different each time with Mistral | 13:27 |
| d0ugal | The test runs are not all exactly the same. | 13:28 |
| d0ugal | That patch changes no Python code. The coverage report should be eactly the same | 13:28 |
| sharatss | d0ugal: exactly. each time we are getting different files or lines | 13:28 |
| d0ugal | so the problem is that the Mistral code has some "randomness" | 13:29 |
| d0ugal | We can talk about it at the meeting in a few hours | 13:31 |
| sharatss | d0ugal: ohh.. yes. will try to get some more info by that time | 13:31 |
| *** jkilpatr has quit IRC | 13:39 | |
| d0ugal | sharatss: I think adding it to mistralclient is okay actually | 13:39 |
| d0ugal | because the code is much simpler I expect it will work | 13:39 |
| d0ugal | so you can ignore my comment on that patch. | 13:39 |
| thrash | d0ugal: are you getting differing results running it locally? | 13:47 |
| d0ugal | thrash: yup | 13:53 |
| d0ugal | make a commit local that does nothing (i.e. update readme) then run `tox -e cover` | 13:54 |
| d0ugal | thrash: this is a common line that is either called or not called. https://github.com/openstack/mistral/blob/master/mistral/db/sqlalchemy/base.py#L149 | 13:59 |
| d0ugal | so I think it is a race condition in the tests that sometimes happens | 13:59 |
| thrash | d0ugal: odd. | 13:59 |
| d0ugal | yeah | 14:00 |
| d0ugal | (╯°□°）╯︵ ┻━┻ | 14:00 |
| * d0ugal gives up | 14:00 | |
| thrash | hehe | 14:00 |
| *** catintheroof has joined #openstack-mistral | 14:04 | |
| *** gongysh has joined #openstack-mistral | 14:11 | |
| *** toure is now known as toure_ | 14:19 | |
| *** toure_ is now known as toure | 14:19 | |
| *** chlong__ has joined #openstack-mistral | 14:57 | |
| openstackgerrit | Merged openstack/python-mistralclient master: Add --utc flag to cron trigger create https://review.openstack.org/441350 | 15:04 |
| *** fultonj has joined #openstack-mistral | 15:06 | |
| *** gongysh has quit IRC | 15:24 | |
| rakhmerov | meeting at #openstack-meeting | 16:00 |
| d0ugal | rbrady, toure, thrash ^ | 16:05 |
| *** thrash is now known as thrash|f00dz | 17:02 | |
| *** fultonj has quit IRC | 17:02 | |
| *** toure is now known as toure|afk | 17:05 | |
| *** rbrady is now known as rbrady-afk | 17:24 | |
| *** jpich has quit IRC | 17:33 | |
| *** thrash|f00dz is now known as thrash | 17:36 | |
| *** jaosorior has quit IRC | 17:58 | |
| *** chlong__ has quit IRC | 18:08 | |
| *** shardy has quit IRC | 18:11 | |
| *** shardy has joined #openstack-mistral | 18:24 | |
| *** thrash is now known as thrash|mtg | 18:33 | |
| *** dprince has quit IRC | 19:02 | |
| *** shardy has quit IRC | 19:11 | |
| *** dprince has joined #openstack-mistral | 19:28 | |
| *** chlong has joined #openstack-mistral | 19:31 | |
| *** thrash|mtg is now known as thrash | 19:33 | |
| *** rbrady-afk is now known as rbrady | 19:35 | |
| *** thrash is now known as thrash|biab | 20:36 | |
| *** chlong has quit IRC | 20:48 | |
| *** dschultz has joined #openstack-mistral | 20:57 | |
| *** jamielennox|away is now known as jamielennox | 21:00 | |
| *** rook has quit IRC | 21:05 | |
| *** rook has joined #openstack-mistral | 21:06 | |
| *** rook is now known as Guest94833 | 21:06 | |
| *** thrash|biab is now known as thrash | 21:11 | |
| *** dprince has quit IRC | 21:17 | |
| *** Guest94833 is now known as rook-new | 21:32 | |
| *** jkilpatr has joined #openstack-mistral | 21:42 | |
| *** catintheroof has quit IRC | 21:44 | |
| *** catintheroof has joined #openstack-mistral | 21:45 | |
| *** jkilpatr has quit IRC | 21:48 | |
| *** catintheroof has quit IRC | 21:49 | |
| *** jkilpatr has joined #openstack-mistral | 21:52 | |
| *** jkilpatr has quit IRC | 21:58 | |
| *** jkilpatr_ has joined #openstack-mistral | 22:01 | |
| *** rbrady is now known as rbrady-afk | 22:05 | |
| *** rook-new is now known as rook | 22:12 | |
| *** sharatss has quit IRC | 22:16 | |
| *** sharatss has joined #openstack-mistral | 22:17 | |
| *** jkilpatr_ has quit IRC | 22:24 | |
| *** jkilpatr_ has joined #openstack-mistral | 22:28 | |
| *** jkilpatr_ has quit IRC | 22:34 | |
| *** jkilpatr_ has joined #openstack-mistral | 22:37 | |
| *** gongysh has joined #openstack-mistral | 22:50 | |
| *** jkilpatr_ has quit IRC | 23:04 | |
| *** jkilpatr_ has joined #openstack-mistral | 23:06 | |
| *** jkilpatr_ has quit IRC | 23:18 | |
| *** jkilpatr_ has joined #openstack-mistral | 23:20 | |
| *** jkilpatr_ has quit IRC | 23:54 | |
| *** gongysh has quit IRC | 23:57 | |
| *** jkilpatr_ has joined #openstack-mistral | 23:58 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!