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

[IMP] tests: improve readability of new Model() in tests #2290

Closed
wants to merge 1 commit into from

Conversation

hokolomopo
Copy link
Contributor

@hokolomopo hokolomopo commented Mar 30, 2023

Description:

Models are often initialized in the tests with a json object to initialize the sheets and cells. The problem is that we tend to define one attribute of the JSON per line of code, no matter the length of the line.

  • this means that initializing a single model takes 7-10 lines of code, which bloats the test file and makes them harder to scroll trough.
  • having a single attribute per line don't make the code of an individual test much more readable IMO.

I propose to discard useless newLines, and try to reduce the lines of code to initialize the model (while still running prettier). This makes the tests more readable.

Odoo task ID : 3256031

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_lt("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Mar 30, 2023

Models are often initialized in the tests with a json object to initialize
the sheets and cells. The problem is that we tend to define one attribute
of the JSON per line of code, no matter the length of the line.

- this means that initializing a single model takes 7-10 lines of code,
which bloats the test file and makes them harder to scroll trough.
- having a single attribute per line don't make the code of an individual
test much more readable IMO.

I propose to discard useless newLines, and try to reduce the lines of
code to initialize the model (while still running prettier). This makes the
tests more readable.

Odoo task 3256031
@hokolomopo hokolomopo force-pushed the master-test-new-model-prettify-adrm branch from 98fb0c7 to ac1abb4 Compare March 31, 2023 09:53
@hokolomopo hokolomopo changed the title [IMP] tests: improve readability of new Model() in tests [IMP] tests: improve readability of new Model() in tests Mar 31, 2023
Copy link
Collaborator

@LucasLefevre LucasLefevre left a comment

Choose a reason for hiding this comment

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

robodoo r+

robodoo pushed a commit that referenced this pull request Mar 31, 2023
Models are often initialized in the tests with a json object to initialize
the sheets and cells. The problem is that we tend to define one attribute
of the JSON per line of code, no matter the length of the line.

- this means that initializing a single model takes 7-10 lines of code,
which bloats the test file and makes them harder to scroll trough.
- having a single attribute per line don't make the code of an individual
test much more readable IMO.

I propose to discard useless newLines, and try to reduce the lines of
code to initialize the model (while still running prettier). This makes the
tests more readable.

Odoo task 3256031

closes #2290

Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
@robodoo robodoo temporarily deployed to merge March 31, 2023 10:21 Inactive
@robodoo robodoo closed this Mar 31, 2023
@robodoo robodoo added the 16.3 label Mar 31, 2023
@fw-bot fw-bot deleted the master-test-new-model-prettify-adrm branch April 14, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants