15:00:47 <yamahata> #startmeeting neutron_northbound
15:00:47 <odl_meetbot> Meeting started Mon Dec 18 15:00:47 2017 UTC.  The chair is yamahata. Information about MeetBot at http://ci.openstack.org/meetbot.html.
15:00:47 <odl_meetbot> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:00:47 <odl_meetbot> The meeting name has been set to 'neutron_northbound'
15:00:54 <yamahata> #chair mpeterson mkolesni poothia
15:00:54 <odl_meetbot> Current chairs: mkolesni mpeterson poothia yamahata
15:01:04 <yamahata> #topic agenda bashing and roll call
15:01:05 <mpeterson> #info mpeterson
15:01:07 <mkolesni> #info mkolesni
15:01:09 <yamahata> #info yamahata
15:01:10 <rajivk> #info rajivk
15:01:17 <yamahata> #chair rajivk
15:01:17 <odl_meetbot> Current chairs: mkolesni mpeterson poothia rajivk yamahata
15:01:32 <yamahata> Today, do we have any additional topics?
15:01:59 <mkolesni> did you guys get approval for ptg?
15:02:03 <yamahata> as patch review, we have several topics on patches.
15:02:16 <mpeterson> the email I've sent about the tests
15:02:33 <yamahata> So far, no progress on travel grant for ptg.
15:02:50 <yamahata> mpeterson: let's discuss it as first topic on patch review.
15:02:56 <mpeterson> sure
15:03:12 <rajivk> no, i won't be able to attend ptg
15:03:15 <mpeterson> no further topics here
15:03:22 <mkolesni> yamahata, do you have estimate when will you know?
15:03:32 <mpeterson> #info discuss the email about the "useless" tests
15:03:51 <yamahata> Maybe in Jan. I'll push it to my manager after new year.
15:04:03 <mkolesni> ok cool
15:04:36 <yamahata> okay, move on
15:04:37 <mkolesni> poothia, are you going to ptg?
15:04:38 <yamahata> #topic Announcements
15:04:55 <poothia_> Not decided yet
15:05:13 <yamahata> schedule: Dec 25 and Jan 1 will be skipped.
15:05:23 <yamahata> The next our irc meeting will be held on Jan 8.
15:06:10 <yamahata> Linux Foundation Networking (LFN) project is announced. OpenDaylight will join LFN.
15:06:32 <yamahata> the next ODL design forum is a part of Open Networking Summit in Los Angels.
15:06:39 <mkolesni> i won't be attending that one
15:06:45 <yamahata> it's a part of LFN sessions.
15:06:49 <manjeets_> #info manjeets
15:06:51 <mkolesni> jhershbe might attend
15:07:03 <mpeterson> #info Due to holidays the next IRC meeting will be on Jan 8
15:07:09 <yamahata> #chair manjeets
15:07:09 <odl_meetbot> Current chairs: manjeets mkolesni mpeterson poothia rajivk yamahata
15:07:13 <yamahata> #chair manjeets_
15:07:13 <odl_meetbot> Current chairs: manjeets manjeets_ mkolesni mpeterson poothia rajivk yamahata
15:08:40 <yamahata> ODL M3 is passed. Isaku will report it to ODL release.
15:08:46 <yamahata> any other announcement?
15:09:13 <mpeterson> #info next ODL design forum is a part of Open Networking Summit in Los Angeles
15:09:22 <mpeterson> #info ODL M3 is passed. Isaku will report it to ODL release.
15:10:25 <yamahata> seems nothing else. move on.
15:10:30 <yamahata> #topic action items from last meeting
15:10:40 <yamahata> we have three items.
15:10:53 <yamahata> all to review https://review.openstack.org/#/c/508826/
15:10:58 <yamahata> mpeterson to update on the tempest jobs progress on next meeting
15:11:05 <yamahata> all to review https://review.openstack.org/#/q/status:open+project:openstack/networking-odl+branch:master+topic:feature/force_maintenance_task
15:11:20 <mkolesni> > all to review https://review.openstack.org/#/c/508826/
15:11:21 <yamahata> we'll discuss patch review later.
15:11:22 <mkolesni> this was done
15:12:03 <yamahata> yeah, cool.
15:12:13 <mpeterson> #info as an update re: tempest jobs, mpeterson is still working on the tempest CI job it's not yet ready but looking good
15:12:47 <rajivk> hey, i had an announcement
15:12:57 <yamahata> rajivk: oh, please go ahead.
15:12:59 <rajivk> i missed that window
15:13:19 <mpeterson> https://review.openstack.org/#/c/528716 Tempest job migration to Zuul v3
15:13:47 <rajivk> I won't be working on networking-odl officially
15:14:06 <rajivk> in my part time, i will be doing it. I have joined a new organization
15:14:30 <yamahata> sad to hear that.
15:14:32 <mpeterson> #info announcement by rajiv: he won't be working on networking-odl officially anymore, only part time
15:14:40 <yamahata> Do you mean new group in the same company?
15:14:56 <rajivk> no different company
15:15:19 <mkolesni> rajivk, sad to see you leave us :(
15:15:32 <manjeets_> :(
15:15:50 <mpeterson> rajivk: too bad, you'll be missed
15:16:36 <mkolesni> i hope you have a good time at your new company
15:17:30 <yamahata> let's move on.
15:17:35 <yamahata> #topic Queens/Oxygen planning
15:17:44 <yamahata> I think no new planning items.
15:17:49 <yamahata> #topic patches/bugs
15:18:00 <yamahata> today we have several items.
15:18:55 <yamahata> One is CI breakage as zuul v3 error, unit test breakage mpeterson pointed out and CI breakage of several tests.
15:18:59 <yamahata> mpeterson: you're on stage
15:19:16 <manjeets_> I submitted a patch to fix unit tests
15:19:34 <manjeets_> https://review.openstack.org/#/c/528020/
15:19:58 <mpeterson> yamahata: yup, I think manjeets_ is more relevant for this :)
15:20:05 <yamahata> manjeets_: that's great.
15:20:11 <mpeterson> yamahata: or are we talking about different things?
15:20:19 <mpeterson> I have the feeling we are talking about two different thigns
15:20:24 <mpeterson> one is the email I've sent
15:20:25 <mkolesni> manjeets_, the patch has review from mpeterson already
15:20:31 <yamahata> I'm talking about the eamil you sent.
15:20:40 <mpeterson> and the other one is the breakage of the CI that happened last thursday
15:20:53 <manjeets_> mkolesni, just saw that today I'll addressed it today
15:20:59 <mkolesni> manjeets_, cool
15:21:11 <mpeterson> okey, so what manjeets_ did is for the second item
15:21:15 <mpeterson> unrelated to that :)
15:21:17 <yamahata> and https://review.openstack.org/#/c/523934/  Fix unit tests for ML2 Mechanism driver
15:21:43 <mpeterson> yamahata: re: my email... those tests with the fix or without the fix aren't really testing anything
15:22:00 <mpeterson> yamahata: therefore they should be removed in favor of tempest
15:22:12 <yamahata> why should they be removed?
15:22:23 <yamahata> unit test and end-to-end test are different class of test cases.
15:22:57 <mkolesni> yamahata, did you read the email mpeterson sent to openstack-dev ?
15:23:02 <yamahata> Yes.
15:23:18 <mkolesni> so he explained there
15:23:22 <yamahata> the unit tests aren't well written. but it doesn't justify to remove them.
15:23:31 <yamahata> They should be fixed. but not removed.
15:23:38 <mkolesni> basically its uses the mech driver but theres no assertion for that
15:23:57 <mkolesni> the base neutron test cant be fixed, its out of scope for us
15:24:15 <mkolesni> how would you suggest to assert that what needed to happen di indeed happen?
15:24:29 <yamahata> we can add such assertion.
15:24:43 <yamahata> neutron test can be fixed. we're  part of neutron.
15:24:56 <mkolesni> yamahata, ok then if you have an idea can you propose a patch to fix this?
15:25:15 <yamahata> just we can fix failing test cases we found.
15:25:21 <yamahata> what prevent us from fixing it?
15:25:21 <mpeterson> yamahata: but I don't think those testcases are meant to assert that
15:25:32 <mkolesni> huh?
15:25:55 <mkolesni> the fix mpeterson did will just have the neutron tests run with the actual mech driver
15:26:19 <mkolesni> which from what we see, in the test itself, is equal to running without the mech driver
15:26:47 <mkolesni> what the tests test is that when you make an api call, it gets registered in the ml2 db tables
15:26:55 <mkolesni> this is asserted by those tests
15:27:09 <mkolesni> it doesnt care what the mech driver does with that
15:27:24 <mkolesni> so we're getting "free tests" that dont actually test anything in our code
15:28:02 <mkolesni> and if they do test something, its more incidental than deliberate since there's no assertions to make sure our code was actually executed
15:28:15 <mkolesni> how do you propose to remedy this situation?
15:28:50 <yamahata> the test cases calls plugin and exercises mech driver.
15:29:34 <mkolesni> and what, in those test cases, or anywhere else, asserts that the mech driver did what it needed to do?
15:31:37 <manjeets_> so we are asserting operation entry in journal table, most of other drivers are register during ml2 init, if the resource is asserted in journal table for those that covers part of mech driver working ?
15:32:16 <mkolesni> manjeets_, where is this assertion of the entry?
15:33:05 <mkolesni> lets say theres a test we get from there that tests create network.. this test creates the network via rest call to neutron, and then asserts it is in the db. there's no assert to check that the network is in the journal table
15:33:18 <manjeets_> mkolesni, I haven't looked at the code for all inherited tests that was my question
15:34:29 <mkolesni> manjeets_, there is no assertion there for anything other than neutron db itself
15:36:16 <yamahata> those testcases calls mech driver. So it makes sense.
15:36:36 <yamahata> It's unfortunate that test cases are poorly designed. But it's another story.
15:36:48 <mpeterson> yamahata: calling the mech driver without asserting is like self.assertTrue(True)
15:36:50 <yamahata> You can raise the issue in the neutron and to go fix it.
15:37:19 <mpeterson> yamahata: the thing is those test cases aren't meant to do that at all, they do what they are supposed already
15:37:24 <yamahata> mpeterson: I see. so? it calls mech driver.
15:37:35 <mpeterson> yamahata: okey, let's put it this way
15:37:50 <mpeterson> yamahata: let's say it calls create_network in the mech driver
15:38:06 <mpeterson> yamahata: in our project it would add a journal entry and that's it
15:38:22 <mpeterson> yamahata: it will succeed because it's not doing anything and the test will succeed
15:38:28 <mpeterson> yamahata: what did it test?
15:38:34 <mkolesni> we raise the issue here.. since it affects us.. these tests give a false sense of coverage
15:38:48 <yamahata> it verified that execution pass can be executed correctly.
15:39:07 <mkolesni> how did it verify that it's correct?
15:39:08 <yamahata> As python is dynamic language, it's non-trivial for the execution path to run.
15:39:15 <mkolesni> maybe a journal entry wasnt created?
15:39:23 <mkolesni> maybe the precommit didnt even run?
15:39:34 <mkolesni> how did it verify anything?
15:39:41 <mkolesni> i fail to understand
15:40:20 <yamahata> With mpeterson patch, we found 6 test cases failure (or more?). What prevent us from fixing it?
15:40:27 <mkolesni> it's like:
15:40:28 <mkolesni> def test() {
15:40:28 <mkolesni> foo(123)
15:40:28 <mkolesni> }
15:40:33 <mkolesni> foo can be a real function
15:40:38 <mkolesni> and also it can be a mock
15:40:51 <mkolesni> in both cases test will be marked as "PASS"
15:41:04 <mkolesni> and in both cases nothing was tested in reality, cause there was no assert
15:41:46 <mkolesni> the matter is not those 6 failures, the matter is generally the value of those tests is questionable
15:42:57 <yamahata> It looks like it needs more time.
15:43:12 <mkolesni> what do you mean?
15:43:14 <yamahata> anyway so far I hesitated to merge other patches.
15:43:18 <mkolesni> how will time help here?
15:43:36 <yamahata> Does it make sense to unblock other patches?
15:43:42 <mkolesni> yes
15:43:54 <yamahata> Okay.
15:43:55 <mkolesni> how will time help however?
15:44:20 <mkolesni> we dont meet for another 3 weeks
15:44:26 <mkolesni> time is not our friend
15:44:35 <mkolesni> we need a decision
15:45:04 <mkolesni> these tests give a false sense of security, like a lock on a door but any key can open it
15:46:14 <yamahata> I'm warried that you're claiming to remove test cases after finding 6 testcases.
15:46:23 <yamahata> without understanding why they fail.
15:46:24 <mkolesni> no thats not related
15:46:32 <mkolesni> we perfectly understand why they fail
15:46:39 <mpeterson> yamahata: I understand perfectly why it fails and I'll write test cases for those
15:46:41 <mkolesni> you misunderstand the situation
15:46:48 <yamahata> so why won't you fix them?
15:46:49 <mpeterson> yamahata: but that's unrelated to the discussion :)
15:47:04 <mkolesni> its not related
15:47:19 <mpeterson> it's not that I won't fix them, it's just it's unrelated
15:47:40 <mkolesni> they fail for a very specific corner case of dependency calculation which causes an exception, they dont 'fail' they 'crash'
15:48:09 <mkolesni> theres a difference between a test failing cause an asserion wasn't met, and a test crashing because of an unexpected bug
15:48:23 <mkolesni> this is the latter and it's not related to the point mpeterson is making
15:48:33 <yamahata> so those test cases are useful to find bugs.
15:48:46 <mkolesni> seriously?
15:48:50 <mkolesni> you really think that?
15:49:12 <mkolesni> youre not even talking on the subject
15:49:40 <yamahata> so will mpeterson fix newly found bugs?
15:49:58 <mkolesni> what does it have to do with the point he was making???
15:50:04 <mkolesni> thats not the point!
15:50:26 <mkolesni> if he fixes or not fixes this corner case doesnt relate to his original point
15:50:49 <mkolesni> this corenr case could've been discovered by any other means, this does not make these tests valuable
15:51:33 <yamahata> Are you planning to add such test cases?
15:51:58 <mkolesni> are you planning to talk to the original point?
15:52:32 <mkolesni> <mpeterson> yamahata: I understand perfectly why it fails and I'll write test cases for those
15:52:35 <mkolesni> <mpeterson> it's not that I won't fix them, it's just it's unrelated
15:52:38 <mkolesni> ???
15:53:35 <mpeterson> yamahata: can you please forget a second about those failures and can we discuss the issue at hand which is 100x more relevant?
15:53:50 <yamahata> by removing test cases, some coverage may be lost. Are you planning to add new test cases to recover them?
15:54:12 <yamahata> I'm afraid of losing coverage.
15:54:56 <manjeets_> can we test the coverage ?, how much of coverage is gone if those are removed
15:54:59 <rajivk> mpeterson, what if we keep those test cases for the time being. It does not impact us anyway
15:55:18 <mpeterson> yamahata: 1) coverage for the sake of coverage is an anti-pattern; 2) the coverage won't be lost because there are test cases that cover those paths already; 3) those 6 issues will be fixed, but it's unrelated
15:55:18 <rajivk> so yamahata concern solve for some time and we can discuss your concern
15:56:09 <yamahata> we're running out of time. we have other issue.
15:56:15 <yamahata> https://review.openstack.org/#/c/528716/
15:56:19 <mpeterson> rajivk: it actually does, they are ~2/3 of all the tests and they are not testing anything, they are masking the real issue that we need to address which is we need to properly test the code
15:56:30 <yamahata> mpeterson: is trying to fix it, right?
15:56:59 <yamahata> oops wrong link
15:57:06 <mpeterson> oh okey
15:57:27 <yamahata> https://review.openstack.org/#/c/518752/
15:57:39 <yamahata> Now you're experimenting it.
15:58:29 <mpeterson> yamahata: not sure I follow
15:59:00 <mpeterson> yamahata: that patch is meant for future work that I'm going to continue doing on the Zuul v3migration
15:59:28 <yamahata> I see.
15:59:51 <yamahata> Now we have zuul -1. and manjeet uploaded a patch to fix unit tests failure.
16:00:01 <yamahata> do we have to address anything else?
16:00:32 <mkolesni> please review mpeterson patch set it's already got many approvals and it's sad to wait
16:00:47 <mpeterson> yamahata: manjeets_ has comments on that fix that we need to address as a whole, but he did correctly discovered the reasons of failrue
16:00:52 <mkolesni> https://review.openstack.org/#/q/status:open+project:openstack/networking-odl+branch:master+topic:feature/force_maintenance_task
16:01:07 <yamahata> mkolesni: you mean force maintenance?
16:01:12 <yamahata> Oh, you're faster than me.
16:01:14 <mkolesni> yes the link ^
16:01:19 <mkolesni> #link https://review.openstack.org/#/q/status:open+project:openstack/networking-odl+branch:master+topic:feature/force_maintenance_task
16:01:22 <yamahata> anything else to discuss today?
16:01:34 <mpeterson> #undo
16:01:34 <odl_meetbot> Removing item from minutes: <MeetBot.ircmeeting.items.Link object at 0x2993e50>
16:01:42 <mkolesni> nope
16:01:54 <yamahata> #topic open mike
16:01:55 <rajivk> no
16:02:07 <poothia_> No
16:02:15 <yamahata> we can continue discussion on mailing list.
16:02:24 <yamahata> happy holidays.
16:02:28 <mkolesni> ok thanks Isaku!
16:02:28 <yamahata> thanks everyone.
16:02:30 <mkolesni> happy holidays
16:02:31 <mpeterson> happy holidays, enjoy!
16:02:32 <manjeets_> i'll address those comments right soon mpeterson
16:02:37 <manjeets_> happy holidays
16:02:42 <yamahata> #topic cookies
16:02:46 <yamahata> #endmeeting