)]}'
{"networking_odl/common/constants.py":[{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"d2c9aa559ba1d634410b81febba4564fb5201ed9","unresolved":false,"context_lines":[{"line_number":107,"context_line":"    ODL_SFC_PORT_PAIR_GROUP: \"%s/%s\" % (ODL_SFC, ODL_SFC_PORT_PAIR_GROUPS)"},{"line_number":108,"context_line":"}"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"# Default QOS RULES"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"DEFAULT_QOS_RULES \u003d {"},{"line_number":113,"context_line":"    qos_consts.RULE_TYPE_BANDWIDTH_LIMIT: {"},{"line_number":114,"context_line":"        qos_consts.MAX_KBPS: {"},{"line_number":115,"context_line":"            \u0027type:range\u0027: [0, db_const.DB_INTEGER_MAX_VALUE]},"},{"line_number":116,"context_line":"        qos_consts.MAX_BURST: {"},{"line_number":117,"context_line":"            \u0027type:range\u0027: [0, db_const.DB_INTEGER_MAX_VALUE]},"},{"line_number":118,"context_line":"        qos_consts.DIRECTION: {"},{"line_number":119,"context_line":"            \u0027type:values\u0027: [constants.EGRESS_DIRECTION]}"},{"line_number":120,"context_line":"    }"},{"line_number":121,"context_line":"}"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_84392b40","line":123,"range":{"start_line":110,"start_character":0,"end_line":123,"end_character":23},"updated":"2018-07-04 08:38:44.000000000","message":"This doesn\u0027t belong here, it\u0027s not common to a lot of drivers but rather specific to only the QoS driver and I think it should remain there..","commit_id":"5a9ff6083a51c66235fba63dc936531421f560e0"}],"networking_odl/common/odl_features.py":[{"author":{"_account_id":22380,"name":"Josh Hershberg","email":"jhershbe@redhat.com","username":"jhershbe"},"change_message_id":"2d068fa64723d35177dcc3e3327e5bc2c090e9cb","unresolved":false,"context_lines":[{"line_number":34,"context_line":"OPERATIONAL_PORT_STATUS \u003d \u0027operational-port-status\u0027"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DEFAULT_FEATURES \u003d {"},{"line_number":37,"context_line":"    odl_const.QOS_RULES: odl_const.DEFAULT_QOS_RULES"},{"line_number":38,"context_line":"}"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"feature_configs \u003d copy.copy(DEFAULT_FEATURES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_5d1ab8c5","line":37,"updated":"2018-05-31 16:57:26.000000000","message":"I think this should not be defined here but rather in qos_driver.py. This is a generic module and should not have any funcitonal specifics.","commit_id":"8c33c747ba1b48369d2938d4267a3c78c6f38990"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"7b48f7bb5722adfcd413023792300ee17e683c25","unresolved":false,"context_lines":[{"line_number":34,"context_line":"OPERATIONAL_PORT_STATUS \u003d \u0027operational-port-status\u0027"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DEFAULT_FEATURES \u003d {"},{"line_number":37,"context_line":"    odl_const.QOS_RULES: odl_const.DEFAULT_QOS_RULES"},{"line_number":38,"context_line":"}"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"feature_configs \u003d copy.copy(DEFAULT_FEATURES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_23af682c","line":37,"in_reply_to":"5f7c97a3_2f0c35eb","updated":"2018-06-07 15:20:42.000000000","message":"Josh, i already agreed that qos default features should not be here in my previous comment. It was the DEFAULT_FEATURES.","commit_id":"8c33c747ba1b48369d2938d4267a3c78c6f38990"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"35b9aeca94b15474bea8e0066c06d17b6e93319c","unresolved":false,"context_lines":[{"line_number":34,"context_line":"OPERATIONAL_PORT_STATUS \u003d \u0027operational-port-status\u0027"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DEFAULT_FEATURES \u003d {"},{"line_number":37,"context_line":"    odl_const.QOS_RULES: odl_const.DEFAULT_QOS_RULES"},{"line_number":38,"context_line":"}"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"feature_configs \u003d copy.copy(DEFAULT_FEATURES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_da500535","line":37,"in_reply_to":"5f7c97a3_5d1ab8c5","updated":"2018-06-01 02:17:02.000000000","message":"yes, i think so too. I also asked this question on your patch.\n\nActually if we don\u0027t do it this way then i don\u0027t understand what is the purpose of this(DEFAULT_FEATURE) variable? We have written many test cases testing DEFAULT_FEATURE and feature_config does not point to the same object.\n\nQuestion is, if it is always going to be {} why it is assigned to feature_configs, it will save us tons of problem.\n\nIs there any scenario, where it\u0027s value will be other than {}, if there is could you please explain it?","commit_id":"8c33c747ba1b48369d2938d4267a3c78c6f38990"},{"author":{"_account_id":22380,"name":"Josh Hershberg","email":"jhershbe@redhat.com","username":"jhershbe"},"change_message_id":"8af503a636283f7cf7f4a3254d06aae2f62b8735","unresolved":false,"context_lines":[{"line_number":34,"context_line":"OPERATIONAL_PORT_STATUS \u003d \u0027operational-port-status\u0027"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DEFAULT_FEATURES \u003d {"},{"line_number":37,"context_line":"    odl_const.QOS_RULES: odl_const.DEFAULT_QOS_RULES"},{"line_number":38,"context_line":"}"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"feature_configs \u003d copy.copy(DEFAULT_FEATURES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_2f0c35eb","line":37,"in_reply_to":"5f7c97a3_da500535","updated":"2018-06-04 05:47:43.000000000","message":"1) DEFAULT_FEATURES is the *empty* initialization of a variable. It is NOT the default feature configuration of the entire networking_odl.\n\n2) DEFAULT_FEATURES could theoretically be renamed NO_FEATURES.\n\n3) For all intents and purposes odl_features will never fail initialization and will always load features either from ODL or from the config files. This is because it either blocks and polls ODL or reads from the config file. As such, anything you put into DEFAULT_FEATURES will, by definition never be used by the application in normal application flow.\n\n4) Finally, even if the above were not enough, it is wrong to put this here from a design perspective. So far this module has been kept generic and in fact is located in the \"common\" directory. It should not have application specific logic in it. If the QoS driver needs some sort of a default, it should be handled in the QoS driver.","commit_id":"8c33c747ba1b48369d2938d4267a3c78c6f38990"}],"networking_odl/qos/qos_driver_v2.py":[{"author":{"_account_id":22380,"name":"Josh Hershberg","email":"jhershbe@redhat.com","username":"jhershbe"},"change_message_id":"2b845c40b964d8e764d139fb1049325dd89d98eb","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    @staticmethod"},{"line_number":79,"context_line":"    def create():"},{"line_number":80,"context_line":"        supported_rules \u003d {}"},{"line_number":81,"context_line":"        for feature in [QOS_DSCP_MARKING, QOS_BANDWIDTH_LIMIT,"},{"line_number":82,"context_line":"                        RULE_TYPE_MINIMUM_BANDWIDTH]:"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if odl_features.has(feature):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_92f5d659","line":81,"updated":"2018-05-29 07:36:30.000000000","message":"We switched to a single feature where the feature configuration will be the supported_rules structure. No?\n\nPlease see: https://git.opendaylight.org/gerrit/#/c/70331/","commit_id":"c1dda204be795e0ea56d93d8ad9bbf2ca51d311d"},{"author":{"_account_id":22380,"name":"Josh Hershberg","email":"jhershbe@redhat.com","username":"jhershbe"},"change_message_id":"2b845c40b964d8e764d139fb1049325dd89d98eb","unresolved":false,"context_lines":[{"line_number":84,"context_line":"            if odl_features.has(feature):"},{"line_number":85,"context_line":"                supported_rules.update(QOS_RULES[feature])"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        return OpenDaylightQosDriver(supported_rules\u003dsupported_rules)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def __init__(self, supported_rules,"},{"line_number":90,"context_line":"                 name\u003d\u0027OpenDaylightQosDriver\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_f2552a51","line":87,"updated":"2018-05-29 07:36:30.000000000","message":"wasn\u0027t the plan to have the supported rules structure returned in the feature configuration?","commit_id":"c1dda204be795e0ea56d93d8ad9bbf2ca51d311d"},{"author":{"_account_id":22380,"name":"Josh Hershberg","email":"jhershbe@redhat.com","username":"jhershbe"},"change_message_id":"65d27e4c50a5a0f70057a9b00b2110c58fd24841","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    }"},{"line_number":48,"context_line":"}"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class OpenDaylightQosDriver(base.DriverBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_50eadf0d","line":50,"updated":"2018-07-29 06:41:46.000000000","message":"I think this should be relocated to odl_features.py to match \"operational_port_status\" which is located there.","commit_id":"86175e8f557f1f7930e097cab8345a40b8bce6f8"},{"author":{"_account_id":22380,"name":"Josh Hershberg","email":"jhershbe@redhat.com","username":"jhershbe"},"change_message_id":"b46e525b2b4fbc3a236dd20655c5a9cd03a6d2a8","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    }"},{"line_number":48,"context_line":"}"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class OpenDaylightQosDriver(base.DriverBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_308ba317","line":50,"in_reply_to":"5f7c97a3_10b3070b","updated":"2018-07-29 08:10:45.000000000","message":"Really? Where do you think operational_port_status belongs? I think there is real value in (a) having the \"odl_features\" namespacing before the name of each feature and (b) having one consolidated list of features that can be negotiated. However, I\u0027m not going to insist so I\u0027ll give you guys the +1 after you find a logical home for operational_port_status.","commit_id":"86175e8f557f1f7930e097cab8345a40b8bce6f8"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"355a6403490a1535e0fe1989e4cce23e568e5572","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    }"},{"line_number":48,"context_line":"}"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class OpenDaylightQosDriver(base.DriverBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_b0233306","line":50,"in_reply_to":"5f7c97a3_10e467de","updated":"2018-07-29 08:58:43.000000000","message":"No, i am not proposing. If this variable is being used at multiple places then i would define a variable in constants\n\nodl_features: {\n  operational_port_status: \u0027\u0027,\n}\n\nbut it is being used at one place. So it does not make sense to put it in constant. Let\u0027s make these changes, when we need them. For now, we don\u0027t need it.","commit_id":"86175e8f557f1f7930e097cab8345a40b8bce6f8"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"c26181a799c68645c6b5546a0d10399c3c6759fd","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    }"},{"line_number":48,"context_line":"}"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class OpenDaylightQosDriver(base.DriverBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_d0c4af43","line":50,"in_reply_to":"5f7c97a3_308ba317","updated":"2018-07-29 08:28:18.000000000","message":"Defining a variable away from the place being used, makes code unreadable. However, if this variable is being used at multiple places across modules then i would prefer it in constants, that\u0027s where i will add odl_feature namespace.\n\nLogical home for operational_port_status is ml2 driver(for now). That is the only place, where it is being used.\n\nI was thinking, putting operational_port_status and qos rules in odl_feature, how does it justify their logical place.","commit_id":"86175e8f557f1f7930e097cab8345a40b8bce6f8"},{"author":{"_account_id":26507,"name":"Michel Peterson","email":"michel@redhat.com","username":"mpeterson"},"change_message_id":"06bcebb5d074b34fcbb30fcbd5d2438df53d33f6","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    }"},{"line_number":48,"context_line":"}"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class OpenDaylightQosDriver(base.DriverBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_d06caf9e","line":50,"in_reply_to":"5f7c97a3_50eadf0d","updated":"2018-07-29 07:12:56.000000000","message":"mmm I disagree. This is a variable particular to a driver. IMHO we should actually move operational_port_status out of there.","commit_id":"86175e8f557f1f7930e097cab8345a40b8bce6f8"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"0c4770c8ab13341a4923880d2b5175e947ce9851","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    }"},{"line_number":48,"context_line":"}"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class OpenDaylightQosDriver(base.DriverBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_10b3070b","line":50,"in_reply_to":"5f7c97a3_d06caf9e","updated":"2018-07-29 07:39:59.000000000","message":"+1 to Michel. odl_feature don\u0027t care, what are the different feature names. Respective driver should manage their own variables.\n\nAgree with operational_port_status to be moved out of there. But that is not the part of this patch.","commit_id":"86175e8f557f1f7930e097cab8345a40b8bce6f8"},{"author":{"_account_id":22380,"name":"Josh Hershberg","email":"jhershbe@redhat.com","username":"jhershbe"},"change_message_id":"9406be1f1cf5fed940d1a994d966901480f06aee","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    }"},{"line_number":48,"context_line":"}"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"QOS_RULES \u003d \u0027qos-rules\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"class OpenDaylightQosDriver(base.DriverBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_10e467de","line":50,"in_reply_to":"5f7c97a3_d0c4af43","updated":"2018-07-29 08:30:10.000000000","message":"I don\u0027t understand, you\u0027re proposing they be put in constants with a namespace? I would go for that.","commit_id":"86175e8f557f1f7930e097cab8345a40b8bce6f8"}],"networking_odl/tests/unit/qos/test_qos_driver_v2.py":[{"author":{"_account_id":26507,"name":"Michel Peterson","email":"michel@redhat.com","username":"mpeterson"},"change_message_id":"77cddb51d2b41c21c8a9d088362bf7a564815746","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    def test_qos_supported_rules_are_fetched_from_odl_feature("},{"line_number":36,"context_line":"            self, mock_odl_features_get_config):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        mocked_qos_features \u003d {\u0027key1\u0027: \u0027value1\u0027}"},{"line_number":39,"context_line":"        mock_odl_features_get_config.return_value \u003d mocked_qos_features"},{"line_number":40,"context_line":"        qos_driver_object \u003d qos_driver.OpenDaylightQosDriver.create()"},{"line_number":41,"context_line":"        self.assertDictEqual(qos_driver_object.supported_rules,"},{"line_number":42,"context_line":"                             mocked_qos_features)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_ff2a8675","line":39,"range":{"start_line":38,"start_character":8,"end_line":39,"end_character":71},"updated":"2018-07-10 08:49:25.000000000","message":"use the config fixture to actually set the config that you want and initialize the features instead of mocking the get_config() method","commit_id":"e9426efcaad08d4f0f38b024a92a6423c55ef890"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"e279d3595f6b96ce48a545f77e5701ef1c01304d","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    def test_qos_supported_rules_are_fetched_from_odl_feature("},{"line_number":36,"context_line":"            self, mock_odl_features_get_config):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        mocked_qos_features \u003d {\u0027key1\u0027: \u0027value1\u0027}"},{"line_number":39,"context_line":"        mock_odl_features_get_config.return_value \u003d mocked_qos_features"},{"line_number":40,"context_line":"        qos_driver_object \u003d qos_driver.OpenDaylightQosDriver.create()"},{"line_number":41,"context_line":"        self.assertDictEqual(qos_driver_object.supported_rules,"},{"line_number":42,"context_line":"                             mocked_qos_features)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_dc1c2f7d","line":39,"range":{"start_line":38,"start_character":8,"end_line":39,"end_character":71},"in_reply_to":"5f7c97a3_168ff122","updated":"2018-07-12 17:54:12.000000000","message":"\u003e if you have to change get_config the end result should be the same still. If the end result changes then the test also needs to be updated.\n\nby get_config get updated, i meant, if get_config interface is not changed but it its definition gets changed. Let\u0027s assume, it calls other method or a network call. In that case, of course, changing get_config test case makes sense but changing create method of qos does not make sense.\n\n\u003e By your reasoning, if you change the OpenDaylightQosDriver and you no longer use odl_features.get_config() then you are again in need to change the test. As such, isolation is relative.\n\nYes, then test case has to be changed, because the code, we are testing, it\u0027s behavior got changed. And we need to capture new behavior. Isolation means the code being tested must be tested in isolation. Not test cases should be isolated from the code being tested. Otherwise what is the benefit of writing test cases, which are executing in itself, in isolation testing anything. Then that is not a test case anymore.\n\n\u003e The get_config is not really part of your test. But a setup of the features is. That\u0027s my reasoning. \n\nMy reasoning is, we should setup things as close as possible to the code being tested. I don\u0027t think, it is a good idea, to setup things far away and believe that the there won\u0027t be any problem to use that setup being made far away from code being tested.","commit_id":"e9426efcaad08d4f0f38b024a92a6423c55ef890"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"9c534bfe2f22a49205157c4a4cf0d09e297effa4","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    def test_qos_supported_rules_are_fetched_from_odl_feature("},{"line_number":36,"context_line":"            self, mock_odl_features_get_config):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        mocked_qos_features \u003d {\u0027key1\u0027: \u0027value1\u0027}"},{"line_number":39,"context_line":"        mock_odl_features_get_config.return_value \u003d mocked_qos_features"},{"line_number":40,"context_line":"        qos_driver_object \u003d qos_driver.OpenDaylightQosDriver.create()"},{"line_number":41,"context_line":"        self.assertDictEqual(qos_driver_object.supported_rules,"},{"line_number":42,"context_line":"                             mocked_qos_features)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_aad94555","line":39,"range":{"start_line":38,"start_character":8,"end_line":39,"end_character":71},"in_reply_to":"5f7c97a3_835aa11b","updated":"2018-07-19 17:40:18.000000000","message":"Mike,\n\nI am not concerned about implementation. I am concerned about correctness and scope of test cases.\n\nHowever i did not understand, why -1 was given by you. There was not reason given. Is it to increase weight of Michel -1? I would have appreciated, if you had given reason of not doing, the way it is done.\n\nI don\u0027t agree, but i have made changes to unblock the patch.","commit_id":"e9426efcaad08d4f0f38b024a92a6423c55ef890"},{"author":{"_account_id":26507,"name":"Michel Peterson","email":"michel@redhat.com","username":"mpeterson"},"change_message_id":"cfbc5657ef794bc528f58c5952f79478cf590381","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    def test_qos_supported_rules_are_fetched_from_odl_feature("},{"line_number":36,"context_line":"            self, mock_odl_features_get_config):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        mocked_qos_features \u003d {\u0027key1\u0027: \u0027value1\u0027}"},{"line_number":39,"context_line":"        mock_odl_features_get_config.return_value \u003d mocked_qos_features"},{"line_number":40,"context_line":"        qos_driver_object \u003d qos_driver.OpenDaylightQosDriver.create()"},{"line_number":41,"context_line":"        self.assertDictEqual(qos_driver_object.supported_rules,"},{"line_number":42,"context_line":"                             mocked_qos_features)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_168ff122","line":39,"range":{"start_line":38,"start_character":8,"end_line":39,"end_character":71},"in_reply_to":"5f7c97a3_8f6a16b3","updated":"2018-07-12 11:03:59.000000000","message":"if you have to change get_config the end result should be the same still. If the end result changes then the test also needs to be updated.\n\nIMHO Mocking should be used as little as possible, if you have other ways to do it. \n\nBy your reasoning, if you change the OpenDaylightQosDriver and you no longer use odl_features.get_config() then you are again in need to change the test. As such, isolation is relative.\n\nThe get_config is not really part of your test. But a setup of the features is. That\u0027s my reasoning. \n\nIDK. Just my 2cents.","commit_id":"e9426efcaad08d4f0f38b024a92a6423c55ef890"},{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"a8fb8a3a8c0d49ec873b4f497a027113da5d5c45","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    def test_qos_supported_rules_are_fetched_from_odl_feature("},{"line_number":36,"context_line":"            self, mock_odl_features_get_config):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        mocked_qos_features \u003d {\u0027key1\u0027: \u0027value1\u0027}"},{"line_number":39,"context_line":"        mock_odl_features_get_config.return_value \u003d mocked_qos_features"},{"line_number":40,"context_line":"        qos_driver_object \u003d qos_driver.OpenDaylightQosDriver.create()"},{"line_number":41,"context_line":"        self.assertDictEqual(qos_driver_object.supported_rules,"},{"line_number":42,"context_line":"                             mocked_qos_features)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_835aa11b","line":39,"range":{"start_line":38,"start_character":8,"end_line":39,"end_character":71},"in_reply_to":"5f7c97a3_dc1c2f7d","updated":"2018-07-16 07:38:08.000000000","message":"This fixture is already set up in networking_odl.tests.unit.test_base_db.ODLBaseDbTestCase which this test class eventually inherits from.\n\nIf you\u0027re concerned about the usage in setUp then just call self.cfg.config in the specific tests you need a specific value.","commit_id":"e9426efcaad08d4f0f38b024a92a6423c55ef890"},{"author":{"_account_id":13912,"name":"rajiv","email":"thenakliman@gmail.com","username":"rajiv.kumar"},"change_message_id":"5c1d7d4c02aed0497e7b73b6ad30690ba4dd4cd3","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    def test_qos_supported_rules_are_fetched_from_odl_feature("},{"line_number":36,"context_line":"            self, mock_odl_features_get_config):"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        mocked_qos_features \u003d {\u0027key1\u0027: \u0027value1\u0027}"},{"line_number":39,"context_line":"        mock_odl_features_get_config.return_value \u003d mocked_qos_features"},{"line_number":40,"context_line":"        qos_driver_object \u003d qos_driver.OpenDaylightQosDriver.create()"},{"line_number":41,"context_line":"        self.assertDictEqual(qos_driver_object.supported_rules,"},{"line_number":42,"context_line":"                             mocked_qos_features)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_8f6a16b3","line":39,"range":{"start_line":38,"start_character":8,"end_line":39,"end_character":71},"in_reply_to":"5f7c97a3_ff2a8675","updated":"2018-07-11 17:20:48.000000000","message":"I found this implementation better, because a unit test should test a unit in isolation. If i do as you say then it won\u0027t be isolated unit.\n\nWhat if due to future requirement, get config has to be changed(it calls other methods) then this test case has to be updated.\n\nHow is that approach better than this one?","commit_id":"e9426efcaad08d4f0f38b024a92a6423c55ef890"},{"author":{"_account_id":26507,"name":"Michel Peterson","email":"michel@redhat.com","username":"mpeterson"},"change_message_id":"77cddb51d2b41c21c8a9d088362bf7a564815746","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        self.assertDictEqual(qos_driver_object.supported_rules,"},{"line_number":42,"context_line":"                             mocked_qos_features)"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @mock.patch.object(odl_features, \u0027get_config\u0027, side_effect\u003dKeyError)"},{"line_number":45,"context_line":"    def test_default_values_for_supported_rules("},{"line_number":46,"context_line":"            self, mock_odl_features_get_config):"},{"line_number":47,"context_line":"        qos_driver_object \u003d qos_driver.OpenDaylightQosDriver.create()"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_9f4b525d","line":44,"range":{"start_line":44,"start_character":4,"end_line":44,"end_character":72},"updated":"2018-07-10 08:49:25.000000000","message":"same","commit_id":"e9426efcaad08d4f0f38b024a92a6423c55ef890"}]}