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.

Sign up for GitHub

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

Jump to bottom

[12.0][IMP] fieldservice: Add tests #257

Merged
merged 1 commit into from
Jul 25, 2019
Merged

[12.0][IMP] fieldservice: Add tests #257

merged 1 commit into from
Jul 25, 2019

Conversation

kittiu
Copy link
Member

@kittiu kittiu commented Jul 3, 2019 •
edited
Loading

Work in progress. I will keep adding more test in each commit. Please feel free to comment/suggest.

Following are models that I think has reasonable logic to get tested

  • revise test_fsm_wizard
  • test_fsm_equipment
  • test_fsm_location
  • test_fsm_person
  • test_fsm_team
  • test_fsm_order

@kittiu kittiu changed the title [12.0][IMP] fieldservice, improve test - work in progress [12.0][IMP] fieldservice, improve test - WIP Jul 3, 2019
@kittiu kittiu mentioned this pull request Jul 3, 2019
Closed
@kittiu kittiu force-pushed the 12.0-imp-fieldservice_add_test branch 2 times, most recently from ffeed8d to 08ab7e6 Compare July 5, 2019 02:50
@kittiu
Copy link
Member Author

kittiu commented Jul 6, 2019

@max3903 while writing test, I see in fsm_order.py still use old style of data string, i.e.,
https://github.com/OCA/field-service/blob/12.0/fieldservice/models/fsm_order.py#L25
Should I open separate PR to change this?

@kittiu
Copy link
Member Author

kittiu commented Jul 6, 2019 •
edited
Loading

@max3903 I found that, this try to ensure that
scheduled_date_end = scheduled_date_start + scheduled_duration
https://github.com/OCA/field-service/blob/12.0/fieldservice/models/fsm_order.py#L196-L212
But when code with write(), it looks complicated. Any reason why we don't use compute field with reverse and store=True for scheduled_date_end?
Can I open separate PR to change this?

@kittiu
Copy link
Member Author

kittiu commented Jul 6, 2019 •
edited
Loading

The test script of this module is pretty much done. Awaiting some comments from following issues,
#263
#264
#265
#266

All of which, if identified, I can help in fixing. Then, I can complete this PR after that. Thanks!

@kittiu kittiu mentioned this pull request Jul 13, 2019
Closed
39 tasks
@max3903 max3903 added this to the 12.0 milestone Jul 17, 2019
@osi-scampbell
Copy link
Contributor

@kittiu Test cases look great, it Travis is showing some errors though. The lint errors for fieldservice_maintenance were fixed here: #270 and the lint errors for fieldservice_repair were fixed here: #269. The other ones are in fsm_order and they just look like a few unneccary imports. Can you take care of those please?

@kittiu kittiu force-pushed the 12.0-imp-fieldservice_add_test branch from de29008 to 4f00d75 Compare July 23, 2019 02:02
@kittiu
Copy link
Member Author

kittiu commented Jul 23, 2019 •
edited
Loading

@osi-scampbell Done as requested. Also rebased and squashed.

@kittiu kittiu force-pushed the 12.0-imp-fieldservice_add_test branch from 4f00d75 to 2c632fb Compare July 23, 2019 02:10
@kittiu kittiu force-pushed the 12.0-imp-fieldservice_add_test branch from 2c632fb to 212c2e0 Compare July 23, 2019 02:10
@max3903
Copy link
Member

max3903 commented Jul 23, 2019

@kittiu Is it still WIP or are you ready to get this merged?

@kittiu
Copy link
Member Author

kittiu commented Jul 25, 2019

This is ready! @max3903

@max3903 max3903 changed the title [12.0][IMP] fieldservice, improve test - WIP [12.0][IMP] fieldservice, improve test Jul 25, 2019
@max3903 max3903 changed the title [12.0][IMP] fieldservice, improve test [12.0][IMP] fieldservice: Add tests Jul 25, 2019
@max3903 max3903 merged commit a7cc768 into OCA:12.0 Jul 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@max3903 max3903 max3903 approved these changes

@osi-scampbell osi-scampbell osi-scampbell approved these changes

Assignees

@kittiu kittiu

Labels
approved enhancement needs review
Projects
None yet
Milestone
12.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kittiu @osi-scampbell @max3903 @OCA-git-bot