Enable test tours only in demo instances.

[yajo]

Usually tests depend on demo data, and they are enabled only in development
environments. By enabling the asset in demo data, you remove them from
production instances and thus increase their performance and security, losing
no features.

This follows what was discussed in OCA/web#402 (diff).

@Tecnativa @lasley

[pedrobaeza]

I prefer not to put test tours on demo, because you don't need to rely on demo data for tours. In fact, I postulate the contrary: don't depend ever on demo data for your tests. You might need to change your tests when on later versions demo data are changed.

[yajo]

I'm sorry but I do not agree with you this time, I think that tests should use demo data, instead. Let me explain the benefits.

Core tests depend on demo data, so not doing that will complicate your life and get no benefit (as tests in production instances will fail anyway).

Creating the required records directly in the test takes almost the same amount of effort as doing that in demo data (in fact, a little bit more), but with the difference that demo data can be human-tested too, while the one created on tests not.

Also, tour tests have to be added to assets_frontend, the ones that get to the final anonymous visitor. Those are bytes and CPU time that they absolutely don't need at all, so saving that will improve their performance and reduce the page size and load time.

[lasley]

Should we instead use the new <odoo> tag?

[yajo]

Not sure when is the moment to update that... I think many people still develop for v8. Maybe when v10 comes out we can update this template more safely.

[oscarolar]

Totally agree with @yajo Tours are not necessary at all on production envs, only when they are on tour mode.

👍

[moylop260]

+1

[pedrobaeza]

We are not talking about the same topic. I'm telling about test tours, not about demo tours. Tests tours should be independent from demo data and to not be loaded with demo BDs, or you won't be able to test your website developments in any environment with real data.

[nhomar]

@pedrobaeza

What @yajo is trying to do is:

IF an asset include a tour which is for a test (frontend test) then it should be good have it in demo instead data due to the fact that this will not work without demo.

Remember Tours are used for demo (show case) and tests (phantom) even if I am not pretty sure this PR technically is correct I like the idea conceptually.

I give my +1 conceptually speaking.

BUT if you want to test your environment with real data and then you need your Tour, put it in data and that's it depends of what kind of test you are doing there is a lot of tests that relly almost completelly in demo data with no posibility of run them in real data (which is conceptually incorreect BTW).

I left open the discussion but this should be nice to hear from odoo itself.

Now it is complex administer the test itself this concept introduce another variable in the already complex system of inheritances in tours.

[rafaelbn]

This PR is 6 months old and has 2 x 👍 from @oscarolar and @moylop260 . Can it be merged then?

[lasley]

I'm for a merge, I like this strategy personally. Sometimes a test is intrinsically linked with the demo data, particularly in the instance of acceptance tests - which is kind of what the tours are IMO.

[pedrobaeza]

I have already said that this will prevent to run JS tests in databases without demo, and that's not what we tend to do with normal tests. Why do it with JS ones?

[lasley]

The tours in some instances could be an acceptance test, highly dependent on data that exists in a static known test set. In some instances this can't co-exist with live data without a lot of changes to make the test dynamic.

Ok then so maybe this is an either, or scenario? Something where the demo should explicitly state only in the use of acceptance tests?

[pedrobaeza]

Yeah, I think so that we can find both scenarios.

[yajo]

Sending tours to end users in production websites impacts performance, which does not happen to normal tests.

OTOH, if your test is for instance a shop buy & checkout tour, you expect that in page 1 of your shop there is the built-in iPad item, for example. Chances for that item not to be in page 1 if you create it in the setUp phase in a production environment are high. UI is only reproducible (and therefore testable) if the data is a given frozen set, which happens only in a vanilla demo instance.

Also, if you have to dedicate 30 minutes to create records in the setUp, you'd probably have them created in 10 minutes in a .csv file and then make the task of functional testing faster and easier too.

And if your app does not behave correctly in production instances, then probably you should try to find the problem there, then create some demo data that imitates that problem, fix it, and test it using that demo data.

An intermediate approach would be finding a way to load demo data in the setUp phase, that would be rolled back later, so you'd have more reproductability, possibility of running tests in production, faster data writing, and faster functional tests. But in the case of tours, I see lots of more advantages in using demo data honestly.

[pedrobaeza]

There are lot of tests that doesn't require that level of specification, but what you want to test are flows. For example, you want to test the sale checkout process. It doesn't matter with which product you do the checkout (maybe only to be an specific type). You only need to make sure the process is completed and the returned page is the expected and so on.

Even more, you should create your own data set before launching the JS tour. Following the example, if you create a product in the test itself, you already know its ID, and you can go to the website product page, locate the "Purchase" button, and continue the rest of the process. All of this without the need of demo data that changes very often from version to version and that makes you to change the test in the migrations with no benefit.

[yajo]

For example, you want to test the sale checkout process. It doesn't matter with which product you do the checkout

Tours are UI tests. UI needs data. You search for "lalala" and have to check no product appears. You search for "ip" and have to check "ipod" and "ipad" appear. You click purchase on both and check that cart has 2 items, and that the amount is correct. Yes, you can do tests like those you say, but current tours are complex enough, and those tests would be much more scope-limited.

demo data that changes very often from version to version and that makes you to change the test in the migrations

At least you don't need to maintain all the data creation code.

with no benefit

There are a few, but I already explained them, so I won't repeat myself. Let's simply wait until this hits -3 or +3 approvals and merge/close. 😁

[pedrobaeza]

OK, here is my 👎

One last point: maintaining data creation code makes traceable the error. Instead, when you use demo data, you have to check in 2 places if the test fails due to the data or due to the code itself.

[lasley]

Just curious, how do deadlocks like this get resolved? Do we look for a net 3 votes on either end?

[pedrobaeza]

I think so

[lasley]

Blazing a trail then woot!

I think your points bring up somewhat of a larger issue though @pedrobaeza. I specifically use demo data in all most of my tests (even Python), because it is something I can guarantee as there & decreases test time nearly tenfold. I also find the XML structure for generation much more clean and maintainable vs. one-off Python record creations & so I tend to think of demo data as somewhat of a test factory.

From your comments, I am gathering that you do not like this practice, so I am wondering what you see as an an advantage when creating all the data locally vs using demo data that is already created (assuming the data is from the same module & you aren't creating some crazy demo dependency chain).

[pedrobaeza]

OK, here are my reasons for not relying on demo data:

• As drafted before, upstream changes in demo data impacts you as well as the changes of code. If you weren't rely on this demo data, the migration would be less painful.
• These changes are also harder to debug, as you have several sources where to look (more even considering possible overwrites in certain fields using XML-ID).
• There are certain problems using demo data on inherited models that add required fields. Required attribute is set as a DB constraint, but you don't have the default available yet. This also happen with data created on the fly on tests, but you can put @common.at_install(False) and @common.post_install(True) to make the test to be executed after all the loading.
• It can have side effects, like this one: 9.0 account bank statement import camt bank-statement-import#79 (comment), where having this demo data (although legit), have made the account module to prevent the installation of the generic chart of accounts and thus all to fail.
• You can inherit better tests and dataset creation methods to get an extensible data framework if needed. This is something not very used till now, but we have already used generic methods that creates the dataset, and extend it in children tests.
• We can even use the same data as demo adding a yaml file that calls the same creation method. But that's something theoretical that I haven't tried...
• Using code, you can mimic some UI processes, like for example calling onchanges to auto-fill certain values. This saves a lot of code and assures you to evolve correctly through versions: if the onchange in the version n + 1 fills other needed fields, you already get it, as you are calling to the corresponding method.
• You can run your tests in any database, without the need to have demo data installed. This is more important that you think, because you can run the tests in your deployed database to check some possible misconfigurations or problems with other installed modules without the need to replicate the same installation. I know that this can be easy to replicate the exact code situation and installed modules with for example Docker, but it's easier to just duplicate the Docker/VM/whatever instance and run the tests there.
• Even more, sometimes you have your database with demo data, but you have touched a bit on it, confirming that demo sales order or so, and this ruin the result of your tests, making you to create again a database.

And this is all I remember right now, but I think it's enough, isn't it?

[yajo]

upstream changes in demo data impacts you

I think @lasley is talking about demo data created in your addon. OTOH I have hit this problem before relying on core demo data, and it's not that hard to fix. You simply point to a different XMLID or copy old demo data in your addon. A matter of minutes. Much faster than creating your own chart of accounts (a real pain), or your own products with templates, variants, rates, prices, descriptions, etc., for example.

we have already used generic methods that creates the dataset, and extend it in children tests

This is not a benefit but a workaround to not using demo data, that would be there always.

Besides, this is not possible in HttpCase (used for tours), because you have to create data inside a with block, and the env disappears outside of it, so yes, you can store the created ID in a test case instance variable, but not the recordset itself (you'd get a MissingError outside the with).

We can even use the same data as demo adding a yaml file that calls the same creation method

Makes more sense IMHO to find a way to reload the demo data file in the test.

Using code, you can mimic some UI processes, like for example calling onchanges to auto-fill certain values

You can call functions in data files too with <function>.

You can run your tests in any database, without the need to have demo data installed

This can be a benefit, indeed, fixed too if we find a way to (re)load demo data at the start of a test. We'd have the benefits of both worlds.

OTOH, this is not always the case. To make reliable tests, you need a reproducible environment. If anything fails in your production database, it is most likely that tests like those you say will fail too, because the problem will be (probably) that you have an untested scenario that happens in production but is not controlled in the test. That is unconnected to the matter here.

Besides, this is less true in tours as I stated above, and there is the performance and bandwidth impact when we talk about tours.

Even more, sometimes you have your database with demo data, but you have touched a bit on it, confirming that demo sales order or so, and this ruin the result of your tests

Again, if we find how to reload demo data before the test, this would be fixed. OTOH, tests are run at install, and bare demo DBs are autospawned by bots, so this is not a very critical problem in our current scenario.

[lmignon]

@pedrobaeza

Demo data can be problematic in some cases but also beneficial in other one.

There are certain problems using demo data on inherited models that add required fields. Required attribute is set as a DB constraint, but you don't have the default available yet. This also happen with data created on the fly on tests, but you can put @common.at_install(False) and @common.post_install(True) to make the test to be executed after all the loading.

When you create records into your tests you are not aware of potential constrains added by addons installed alongside your addon. In some case demo data solve potential failures when they are updated in the module adding the new constrains.

It can have side effects, like this one: OCA/bank-statement-import#79 (comment), where having this demo data (although legit), have made the account module to prevent the installation of the generic chart of accounts and thus all to fail.

You are right, If your demo data create account.account records it will prevent the normal installation a a default chart of account in the post_install process of account and the installation will fail on the creation of the demo data for invoice that requires an installed chart of account.

You can inherit better tests and dataset creation methods to get an extensible data framework if needed. This is something not very used till now, but we have already used generic methods that creates the dataset, and extend it in children tests.

It's partially true. In a lot of cases, you are not aware of addons installed alongside of the addons tested. It's always the case when tests are launched for all the addons in a directory at once (The MQT way) . In such a case, the order in which the tests are ran is defined by odoo and your tests will not extend the generic methods defined by the addons which it does not depend on.

You can run your tests in any database, without the need to have demo data installed. This is more important that you think, because you can run the tests in your deployed database to check some possible misconfigurations or problems with other installed modules without the need to replicate the same installation. I know that this can be easy to replicate the exact code situation and installed modules with for example Docker, but it's easier to just duplicate the Docker/VM/whatever instance and run the tests there.
Even more, sometimes you have your database with demo data, but you have touched a bit on it, confirming that demo sales order or so, and this ruin the result of your tests, making you to create again a database.

A unittest is valid ONLY if it rely on a reproducible environment. In Odoo tests are designed to be run on an empty database. Even if a test is successful on an existing database, it can be a false positive if the data are modified in a way that hide a real issue.

We heavily write unittest in our projects and it's true that the use of demo data can be painful in some cases. The problem when using demo data is to expect at the beginning of your tests that these data are in a specific state (initialized with specific values required by your test). A good practice is to enforce the values expected on your demo data in the setup of your tests to be sure that you are in a predictable state. We must also take care to avoid the creation of account.account in these demo data. But despite these limitations I continue to think that they offer more advantages than disadvantages? We also develop tests using memory records (self.env['my.model'].new({})) The advantage of this approach is that you don't need to specify/know all the required attributes of your model instance but you just give the required dataset to validate your code. But you must know when you write this kind of tests that the ORM cache is invalidated in case of error or explicit calls to write or create or unlink.

[tarteo]

👍

[yajo]

Please can you use the GH review tool to approve/reject? It's easier to count that way 😉

[tarteo]

@yajo done :)

[pedrobaeza]

@yajo are you going to continue with this? It's need a refresh to latest versions

[yajo]

Updated, this is ready to merge IMHO.