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

Fixing issues of tour test for awesome_tshirt #1

Open
wants to merge 56 commits into
base: master
Choose a base branch
from

Conversation

BT-rmartin
Copy link

  • Fixing issue in the workflow. When trying to run this tour test I got an issue because the new order was not found. By debugging it the problem was that as we weren't clicking on the "Order" button, it wasn't calling to the other controller /awesome_tshirt/validate_order where the order is really created. Now it clicks the button and in the next endpoint it finds the "Make another order" button to switch to the /web. Therefore now the order is created
  • Fixing typo in the class o_list_number. Otherwise despite the order was created it couldn't find and element in the UI with that content, as it was not a class

fdardenne and others added 16 commits October 9, 2022 20:32
- Fixing issue in the workflow. When trying to run this tour test I got an issue because the new order was not found. 
By debugging it the problem was that as we weren't clicking on the "Order" button, it wasn't calling to the other controller /awesome_tshirt/validate_order where the order is really created. Now it clicks the button and in the next endpoint it finds the "Make another order" button to switch to the /web. Therefore now the order is created
- Fixing typo in the class o_list_number. Otherwise despite the order was created it couldn't find and element in the UI with that content, as it was not a class
@BT-rmartin
Copy link
Author

@ged-odoo Continuing with the exercises of the SmartClasses I found an issue here.
Please, review it and let me know your opinion.

By the way, I have a more general question.
With the tour tests, should we do the validations/asserts as you did here searching for the expected results in the UI, or it would be also valid solution just doing the first part of the tour sending info through the endpoint, and then check that the order was created in Python as follows?
image

In my opinion, tours should be more close to Selenium, and there we should check all results in the UI.
Otherwise, if we want to check results from Python, there would be no need for a tour test, just with an HttpCase test calling directly to the endpoint and checking the result would be enough (no tour).

Perhaps the point there is that we should just do tours in case there is some logic in JS in the endpoint, but if the endpoint is just a plain representation of a controller, we can simply do it with HttpCase.

Looking forward to hear from your expertise.

Thanks a lot in advance.

@BT-rmartin
Copy link
Author

image
Another issue is that to run the testing tours the buttons are not visible.
You cannot resize the pop-up and then till you select text and you are able to move to the right, you don't see the buttons.
image

The popup should be able to resize or at least show an horizontal scroll bar to be aware there are buttons over there

@BT-rmartin
Copy link
Author

@ged-odoo @fdardenne @aab-odoo Could you please double check this?

@fdardenne
Copy link
Contributor

fdardenne commented Oct 21, 2022

Hello @BT-rmartin thank you for your PR,

@ged-odoo Continuing with the exercises of the SmartClasses I found an issue here. Please, review it and let me know your opinion.

By the way, I have a more general question. With the tour tests, should we do the validations/asserts as you did here searching for the expected results in the UI, or it would be also valid solution just doing the first part of the tour sending info through the endpoint, and then check that the order was created in Python as follows? image

In my opinion, tours should be more close to Selenium, and there we should check all results in the UI. Otherwise, if we want to check results from Python, there would be no need for a tour test, just with an HttpCase test calling directly to the endpoint and checking the result would be enough (no tour).

Perhaps the point there is that we should just do tours in case there is some logic in JS in the endpoint, but if the endpoint is just a plain representation of a controller, we can simply do it with HttpCase.

Looking forward to hear from your expertise.

Thanks a lot in advance.

In my opinion the check that the data has been inserted in the database should be done at the python level (here the verification is probably done at the JS level for educational purposes).
This way the test is more robust because in JS the name of the class can always change or even the representation of the decimal in the tree view. And also that the tree view <--> database is already tested in Odoo.
You're probably right about the fact that you can test the controller with HTTPCase. But how can you detect if someone wrongly changes the html form and the call doesn't match the API of your controller anymore?

image Another issue is that to run the testing tours the buttons are not visible. You cannot resize the pop-up and then till you select text and you are able to move to the right, you don't see the buttons. image

The popup should be able to resize or at least show an horizontal scroll bar to be aware there are buttons over there

I think this problem should be reported in an issue in Odoo and not in this training repo

Copy link
Contributor

@fdardenne fdardenne left a comment

Choose a reason for hiding this comment

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

Looks good to me except for the string of content that should be changed 👍

awesome_tshirt/static/tests/tours/order_flow.js Outdated Show resolved Hide resolved
@BT-rmartin
Copy link
Author

BT-rmartin commented Oct 27, 2022

First of all @fdardenne thanks a lot for replying.

In my opinion the check that the data has been inserted in the database should be done at the python level (here the verification is probably done at the JS level for educational purposes).

But then, if you need to verify intermediate steps of your tour, you need to create several smaller tours, call them, and do the checks in Python in the middle right?
I agree it's less error-prone checking in Python, as in the UI there could be changes of the class of the element, problems of translations, etc. However, I guess this kind of tests is to guarantee the result of the UI, and then, checking in Python that something was created doesn't guarantee that in the UI it will appear (perhaps you have filters applied, or some other crappy module has changed the view, etc). It's then just a matter of finding a way to uniquely identify robustly an element in the UI.

You're probably right about the fact that you can test the controller with HTTPCase. But how can you detect if someone wrongly changes the html form and the call doesn't match the API of your controller anymore

Regarding this, you are right, if the target of the form changes we are not testing that the workflow succeeds for the end user.

In other words, I would like to know when it's better to do tour tests, when testing controllers with HttpCase, when just QUnit, when just Unit tests. And the answer will be probably "it depends", but I would appreciate to get some examples or your advice.

I think this problem should be reported in an issue in Odoo and not in this training repo
Reported odoo/odoo#104321

@fdardenne
Copy link
Contributor

Hello @BT-rmartin ,
Sorry for the delay.

For the moment the solutions are merged in a private repo and are then cherry-picked on this public repo.

All of this is temporary, we want to transfer the code and solutions in a new repo in the Odoo organization because we are rewriting the tutorial in the Odoo documentation.

Do you agree that we squash your commit in the right commit in the private repo ? We'll add your name in the co-author

@BT-rmartin
Copy link
Author

@fdardenne Sure, no problem. Could you please answer when it's better to do tour tests, when testing controllers with HttpCase, when just QUnit, when just Unit tests. And the answer will be probably "it depends", but I would appreciate to get some examples or your advice.

@fdardenne
Copy link
Contributor

fdardenne commented Nov 17, 2022

@fdardenne Sure, no problem. Could you please answer when it's better to do tour tests, when testing controllers with HttpCase, when just QUnit, when just Unit tests. And the answer will be probably "it depends", but I would appreciate to get some examples or your advice.

Thanks 👍
QUnit tests: To test each units of your component. Example: clicking on increment in the counter component actually shows that the text is modified.
Tour: To test the integration of your components into the Odoo environment. Example: if your component or view is part of a flow, you want to test if everything is well coordinated (example: did your component passed a good context to the next view/client action ?)
The two above are not exclusive, they are complementary.

For controllers:
HTTPCase:

  • to setup your tour (like creating objects needed by your tour to properly work)
  • to verify if your controller actually create the object in the database. If your list is standard (no search/domain by default,...), you actually know Odoo already tested the list view. But if your view is not standard, a tour in your list view can be complementary to your HTTPCase check.
  • to verify if the status code of your controller is good and if the links are not broken

I'm not a testing expert, this is my opinion and hope that this is useful to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants