[IMP] hr_holidays: Improve business tests #28323
Conversation
| 'date_to': datetime.today(), | ||
| 'number_of_days': 1, | ||
| }) | ||
| self.env['hr.leave'].search([('name', '=', 'Hol10')]).unlink() |
There was a problem hiding this comment.
If creation failed then no need to unlink the record, moreover in a test everything is rollbacked at the end of the method
|
|
||
| from odoo.addons.hr_holidays.tests.common import TestHrHolidaysBase | ||
|
|
||
| class TestAccrualRequests(TestHrHolidaysBase): |
There was a problem hiding this comment.
I think we could rename this test to something like TestLeaveRequests or TestHrLeave
|
|
||
| @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') | ||
| def test_accrual_validity_time_valid(self): | ||
| """ Employee ask leav during a valid validity time """ |
| }) | ||
|
|
||
| @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') | ||
| def test_accrual_no_days(self): |
There was a problem hiding this comment.
This is not a test for accrual, I suggest to rename it test_limited_type_no_days
| }) | ||
|
|
||
| @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') | ||
| def test_accrual_enough_days_left(self): |
There was a problem hiding this comment.
I suggest naming it test_limited_type_days_left
| self._check_holidays_status(holiday_status, 2.0, 2.0, 0.0, 0.0) | ||
|
|
||
| @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') | ||
| def test_accrual_hr_manager_not_enough_days_left(self): |
There was a problem hiding this comment.
This is testing almost the same thing as above, we should change this one to test if at writing on a confirmed request you cannot use more days than what the employee has.
We can test this in the write method and in the action_confirm method which should both raise an error in this case
Hence I suggest to rename this into test_limited_write_more_days
| @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') | ||
| def test_accrual_hr_manager_not_enough_days_left(self): | ||
| """ Employee creates a leave request in a limited category and has enough days left. | ||
| After a HR manager try to modify the number of days but has not enough days """ |
There was a problem hiding this comment.
This docstring is quite obscure, we should rewrite it I think of something like :
""" Employee creates creates a leave request in a limited category with enough remaining leaves. Manager tries to rewrite the number of days of the request to be bigger than the remaining leaves, this should fail at write and at action_confirm"""
| """ % (newdate, id)) | ||
|
|
||
| @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') | ||
| def test_accrual_interlapping(self): |
There was a problem hiding this comment.
We should rename this to test_overlapping_requests
| hol.sudo(self.user_hrmanager_id).action_confirm() | ||
|
|
||
| @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') | ||
| def test_accrual_validity_time_valid(self): |
There was a problem hiding this comment.
I suggest renaming this to test_type_validity and merging it with the next one
| @@ -118,31 +103,6 @@ def _check_holidays_status(holiday_status, ml, lt, rl, vrl): | |||
| # Case2: limited type of leave request | |||
There was a problem hiding this comment.
We could split this into two flows, using the comments, you'd have one testing in the case of an hr.leave.type which would not be limited and another with limited hr.leave.type
jbm-odoo
force-pushed
the
12.0-hr-holidays-business-tests-jbm
branch
from
a8364fd
to
b3aba9f
Compare
robodoo
added
CI 🤖
|
@robodoo r+ |
|
I'm sorry, @RomainLibert. I'm afraid I can't do that. |
The tests have been detroyed while merging a refactoring for 12.0. This commit reintroduces unit tests to checks each business cases from "The Flow" into python tests.
tivisse
force-pushed
the
12.0-hr-holidays-business-tests-jbm
branch
from
b3aba9f
to
db6a7e6
Compare
|
@robodoo r+ |
The tests have been detroyed while merging a refactoring for 12.0. This commit reintroduces unit tests to checks each business cases from "The Flow" into python tests. closes #28323
|
Merged, thanks! |
Description of the issue/feature this PR addresses:
The tests have been detroyed while merging half digested crap for the 12.0
Desired behavior after PR is merged:
new Unit tests to checks each business cases from "The Flow" into python tests.
--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr