Skip to content
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

11.0 fix employee firstname test #427

Merged
merged 2 commits into from
Mar 7, 2018

Conversation

feketemihai
Copy link
Member

No description provided.

@feketemihai
Copy link
Member Author

@pedrobaeza Can i have some help please, i've pushed #406 into main branch( but by mistake i choose merge commits, not squash), and now the tests fail, but i don't get it why, on the PR everything was ok, all were green, but now, i really don't know why...

@feketemihai
Copy link
Member Author

feketemihai commented Jan 27, 2018

Stange is that locally i don't have any error on tests, both modules...with Odoo latest revision..

@feketemihai feketemihai force-pushed the 11.0-fix_employee_firstname_test branch from e616d38 to d8bb932 Compare January 27, 2018 14:40
@feketemihai feketemihai added this to the 11.0 milestone Jan 27, 2018
@feketemihai feketemihai force-pushed the 11.0-fix_employee_firstname_test branch 3 times, most recently from 8d258d6 to 499a6f3 Compare January 29, 2018 06:12
Update test.

Update test.

Update test.

Update test.
Copy link
Member

@hbrunn hbrunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could have also just explicitly called the decorated function - we're not testing if the framework works, but if the functions we use do what they should. But I think it's okay the way it is

@feketemihai
Copy link
Member Author

@pedrobaeza @espo-tony Can i have your review here, since the module is merged and we have a bug, and other PR's are red without this..

@@ -46,12 +46,22 @@ def test_onchange(self):
"""
Validate the get_name method is not failing
"""
field_onchange = self.employee1_id._onchange_spec()
field_onchange = self.employee_model._onchange_spec()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are running the onchange over an empty recordset, which is for sure invalid. If you want to set some kind of default, do it over a NewId recordset (created with new() method)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you are making some assert after this call, so it's not correct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which line? Any way, what I'm proposing is

field_onchange = self.employee_model.new({})
field_onchange._onchange_spec()
self.assertEqual(field_onchange.get('firstname'), '1')
self.assertEqual(field_onchange.get('lastname'), '1')

It's for assuring no weird behavior is got over versions.

@pedrobaeza pedrobaeza merged commit c3d98bd into OCA:11.0 Mar 7, 2018
sambarros pushed a commit to sambarros/hr that referenced this pull request Jul 26, 2018
…rs_change_date_value_when_Payment_Execution_Date_Type_==_Fixed_Date

[BIZ-1503][IMP] Set move date equal to Payment Execution when Payment…
Mraimou pushed a commit to camptocamp/hr that referenced this pull request Sep 26, 2020
luistorresm pushed a commit to vauxoo-dev/hr that referenced this pull request Oct 12, 2020
komsan-S pushed a commit to ecosoft-odoo/hr that referenced this pull request Nov 10, 2020
luistorresm pushed a commit to vauxoo-dev/hr that referenced this pull request Oct 18, 2021
luistorresm pushed a commit to vauxoo-dev/hr that referenced this pull request Oct 19, 2021
luistorresm pushed a commit to vauxoo-dev/hr that referenced this pull request Nov 5, 2022
andreagidaltig pushed a commit to vauxoo-dev/hr that referenced this pull request Nov 6, 2023
andreagidaltig pushed a commit to vauxoo-dev/hr that referenced this pull request Nov 8, 2023
luisg123v pushed a commit to Vauxoo/hr that referenced this pull request Nov 14, 2023
andreagidaltig pushed a commit to vauxoo-dev/hr that referenced this pull request Nov 16, 2023
andreagidaltig pushed a commit to vauxoo-dev/hr that referenced this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants