-
-
Notifications
You must be signed in to change notification settings - Fork 152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MIG]Migrated Business Requirement Deliverable Resource Template from 8.0 to 10.0. #236
[MIG]Migrated Business Requirement Deliverable Resource Template from 8.0 to 10.0. #236
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@seb-elico @victormartinelicocorp |
@elicoidal Need #223 #224 merged and this re-base as soon is done. Any later issue could not be discovered now. |
@@ -24,7 +24,7 @@ def setUp(self): | |||
'factor': 1, | |||
'uom_type': 'reference', | |||
'rounding': 0.000001}) | |||
self.ProductS = self.env.ref('product.product_product_consultant') | |||
self.ProductS = self.env.ref('product.service_order_01') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why changing the product?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seb-elico The product wasn't exist in v10 product module. So we changed it by other service type of product.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sudhir-serpentcs @elicoidal @seb-elico
UT was designed before just to pass coveralls.
Need to improve quality of Test.
Extend the BRD test so the DL product change should be covered by the tested before this module.
Now specific here is the fact we need test whether the DL have generated the RL with same information as the product template resources.
For instance:
def test_product_id_onchnage(self):
(by the way name is wrong)
def test_product_id_onchange(self):
A)Setup the condition to test. (done in def setUp)
B)Execute the function. (product_id_change)
C)Check with assert the value or values expected.
Agreed with @victormartinelicocorp |
@sudhir-serpentcs Let's try to finish this today 😄 |
What is the situation here? |
@victormartinelicocorp Can you review the tests so that we can move forward? |
@sudhir-serpentcs Travis first error is on here: ======== Testing test_flake8 ======== business_requirement_deliverable_resource_template/tests/test_br_deliverable.py:5:1: F401 'odoo.exceptions.ValidationError' imported but unused @elicoidal Others errors cause dependency on BRD. So need to rebase this PR with #224 once is merged |
@victormartinelicocorp can you recheck your review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move all _demo.xml
files into folder /demo as OCA guidelines suggest.
https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#directories
Functional tests are OK |
@victormartinelicocorp |
@sudhir-serpentcs @YogeshMahera-SerpentCS Could have a check? cc @elicoidal |
d7f073a
to
519a1f9
Compare
@victormartinelicocorp |
… 8.0 to 10.0. (OCA#236) * [MIG]Migrated Business Requirement Deliverable Resource Template from 8.0 to 10.0. * [IMP]Added Test-cases and Improved test-cases code * [IMP]Improved code for demo data * [IMP]Removed unused import statement * [REM/ADD]Moved demo data files from data folder to demo folder
Issue #48