Skip to content

Commit

Permalink
[IMP] tests: improve readability of new Model() in tests
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
hokolomopo committed Mar 31, 2023
1 parent d303699 commit 2e02cfc
Show file tree
Hide file tree
Showing 19 changed files with 167 additions and 1,093 deletions.
9 changes: 1 addition & 8 deletions tests/plugins/autofill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,14 +525,7 @@ describe("Autofill", () => {
});

test("autofill with merge greater than the grid size", () => {
model = new Model({
sheets: [
{
colNumber: 1,
rowNumber: 5,
},
],
});
model = new Model({ sheets: [{ colNumber: 1, rowNumber: 5 }] });
merge(model, "A1:A2");
autofill("A1:A2", "A5");
expect(getMergeCellMap(model)).toEqual(XCToMergeCellMap(model, ["A1", "A2", "A3", "A4"]));
Expand Down
36 changes: 4 additions & 32 deletions tests/plugins/automatic_sum.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,57 +480,29 @@ describe("automatic sum", () => {
});

test("sum data in the last row", () => {
const model = new Model({
sheets: [
{
colNumber: 1,
rowNumber: 2,
},
],
});
const model = new Model({ sheets: [{ colNumber: 1, rowNumber: 2 }] });
setCellContent(model, "A1", "4");
setCellContent(model, "A2", "4");
automaticSum(model, "A1:A2");
});

test("sum data horizontally in the bottom right corner", () => {
const model = new Model({
sheets: [
{
colNumber: 2,
rowNumber: 1,
},
],
});
const model = new Model({ sheets: [{ colNumber: 2, rowNumber: 1 }] });
setCellContent(model, "A1", "4");
setCellContent(model, "B1", "4");
automaticSum(model, "A1:B1");
});

test("not at the end but no next empty row", () => {
const model = new Model({
sheets: [
{
colNumber: 1,
rowNumber: 3,
},
],
});
const model = new Model({ sheets: [{ colNumber: 1, rowNumber: 3 }] });
setCellContent(model, "A1", "4");
setCellContent(model, "A2", "4");
setCellContent(model, "A3", "4");
automaticSum(model, "A1:A2");
});

test("not at the end but no next empty col", () => {
const model = new Model({
sheets: [
{
colNumber: 3,
rowNumber: 1,
},
],
});
const model = new Model({ sheets: [{ colNumber: 3, rowNumber: 1 }] });
setCellContent(model, "A1", "4");
setCellContent(model, "B1", "4");
setCellContent(model, "C1", "4");
Expand Down
117 changes: 16 additions & 101 deletions tests/plugins/clipboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,7 @@ describe("clipboard", () => {

test("can copy and paste merged content", () => {
const model = new Model({
sheets: [
{
id: "s1",
colNumber: 5,
rowNumber: 5,
merges: ["B1:C2"],
},
],
sheets: [{ id: "s1", colNumber: 5, rowNumber: 5, merges: ["B1:C2"] }],
});
copy(model, "B1");
paste(model, "B4");
Expand All @@ -309,14 +302,7 @@ describe("clipboard", () => {

test("can cut and paste merged content", () => {
const model = new Model({
sheets: [
{
id: "s2",
colNumber: 5,
rowNumber: 5,
merges: ["B1:C2"],
},
],
sheets: [{ id: "s2", colNumber: 5, rowNumber: 5, merges: ["B1:C2"] }],
});
cut(model, "B1:C2");
paste(model, "B4");
Expand All @@ -337,11 +323,7 @@ describe("clipboard", () => {
id: "s1",
colNumber: 5,
rowNumber: 5,
cells: {
A1: { content: "merge" },
C1: { content: "a" },
D2: { content: "a" },
},
cells: { A1: { content: "merge" }, C1: { content: "a" }, D2: { content: "a" } },
merges: ["A1:B2"],
},
],
Expand All @@ -357,17 +339,8 @@ describe("clipboard", () => {
test("copy/paste a merge from one page to another", () => {
const model = new Model({
sheets: [
{
id: "s1",
colNumber: 5,
rowNumber: 5,
merges: ["B2:C3"],
},
{
id: "s2",
colNumber: 5,
rowNumber: 5,
},
{ id: "s1", colNumber: 5, rowNumber: 5, merges: ["B2:C3"] },
{ id: "s2", colNumber: 5, rowNumber: 5 },
],
});
const sheet2 = "s2";
Expand All @@ -383,17 +356,8 @@ describe("clipboard", () => {
test("copy/paste a formula that has no sheet specific reference to another", () => {
const model = new Model({
sheets: [
{
id: "s1",
colNumber: 5,
rowNumber: 5,
cells: { A1: { content: "=A2" } },
},
{
id: "s2",
colNumber: 5,
rowNumber: 5,
},
{ id: "s1", colNumber: 5, rowNumber: 5, cells: { A1: { content: "=A2" } } },
{ id: "s2", colNumber: 5, rowNumber: 5 },
],
});

Expand Down Expand Up @@ -1128,14 +1092,7 @@ describe("clipboard", () => {
});

test("can copy a cell with a conditional format and paste value only", () => {
const model = new Model({
sheets: [
{
colNumber: 5,
rowNumber: 5,
},
],
});
const model = new Model({ sheets: [{ colNumber: 5, rowNumber: 5 }] });
setCellContent(model, "A1", "1");
setCellContent(model, "A2", "2");
setCellContent(model, "C1", "1");
Expand Down Expand Up @@ -1549,14 +1506,7 @@ describe("clipboard", () => {
});

test("can copy and paste a conditional formatted cell", () => {
const model = new Model({
sheets: [
{
colNumber: 5,
rowNumber: 5,
},
],
});
const model = new Model({ sheets: [{ colNumber: 5, rowNumber: 5 }] });
setCellContent(model, "A1", "1");
setCellContent(model, "A2", "2");
setCellContent(model, "C1", "1");
Expand All @@ -1581,14 +1531,7 @@ describe("clipboard", () => {
expect(getStyle(model, "C2")).toEqual({});
});
test("can cut and paste a conditional formatted cell", () => {
const model = new Model({
sheets: [
{
colNumber: 5,
rowNumber: 5,
},
],
});
const model = new Model({ sheets: [{ colNumber: 5, rowNumber: 5 }] });
setCellContent(model, "A1", "1");
setCellContent(model, "A2", "2");
setCellContent(model, "C1", "1");
Expand All @@ -1612,14 +1555,7 @@ describe("clipboard", () => {
});

test("can copy and paste a conditional formatted zone", () => {
const model = new Model({
sheets: [
{
colNumber: 5,
rowNumber: 5,
},
],
});
const model = new Model({ sheets: [{ colNumber: 5, rowNumber: 5 }] });
setCellContent(model, "A1", "1");
setCellContent(model, "A2", "2");
const sheetId = model.getters.getActiveSheetId();
Expand Down Expand Up @@ -1652,14 +1588,7 @@ describe("clipboard", () => {
});

test("can cut and paste a conditional formatted zone", () => {
const model = new Model({
sheets: [
{
colNumber: 5,
rowNumber: 5,
},
],
});
const model = new Model({ sheets: [{ colNumber: 5, rowNumber: 5 }] });
setCellContent(model, "A1", "1");
setCellContent(model, "A2", "2");
const sheetId = model.getters.getActiveSheetId();
Expand Down Expand Up @@ -1687,16 +1616,8 @@ describe("clipboard", () => {
test("can copy and paste a conditional formatted cell to another page", () => {
const model = new Model({
sheets: [
{
id: "s1",
colNumber: 5,
rowNumber: 5,
},
{
id: "s2",
colNumber: 5,
rowNumber: 5,
},
{ id: "s1", colNumber: 5, rowNumber: 5 },
{ id: "s2", colNumber: 5, rowNumber: 5 },
],
});
setCellContent(model, "A1", "1");
Expand Down Expand Up @@ -1725,14 +1646,8 @@ describe("clipboard", () => {
test("can cut and paste a conditional formatted cell to another page", () => {
const model = new Model({
sheets: [
{
colNumber: 5,
rowNumber: 5,
},
{
colNumber: 5,
rowNumber: 5,
},
{ colNumber: 5, rowNumber: 5 },
{ colNumber: 5, rowNumber: 5 },
],
});
const sheet1 = model.getters.getSheetIds()[0];
Expand Down
16 changes: 4 additions & 12 deletions tests/plugins/conditional_formatting.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,9 +489,7 @@ describe("conditional format", () => {
{
colNumber: 7,
rowNumber: 4,
cells: {
C3: { content: "42" },
},
cells: { C3: { content: "42" } },
conditionalFormats: [
{ id: "1", ranges: ["A1:A1"], rule },
{ id: "2", ranges: ["B2:B2"], rule },
Expand Down Expand Up @@ -523,9 +521,7 @@ describe("conditional format", () => {
{
colNumber: 4,
rowNumber: 7,
cells: {
C3: { content: "42" },
},
cells: { C3: { content: "42" } },
conditionalFormats: [
{ id: "1", ranges: ["A1:A1"], rule },
{ id: "2", ranges: ["B2:B2"], rule },
Expand Down Expand Up @@ -561,9 +557,7 @@ describe("conditional format", () => {
{
colNumber: 3,
rowNumber: 4,
cells: {
B4: { content: "42" },
},
cells: { B4: { content: "42" } },
conditionalFormats: [
{ id: "1", ranges: ["A1:C1"], rule },
{ id: "2", ranges: ["A2:B2"], rule },
Expand All @@ -590,9 +584,7 @@ describe("conditional format", () => {
{
colNumber: 4,
rowNumber: 3,
cells: {
D2: { content: "42" },
},
cells: { D2: { content: "42" } },
conditionalFormats: [
{ id: "1", ranges: ["A1:A3"], rule },
{ id: "2", ranges: ["B1:B2"], rule },
Expand Down
13 changes: 3 additions & 10 deletions tests/plugins/core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,7 @@ describe("core", () => {
});

test("xc is expanded to overlapping merges", () => {
const model = new Model({
sheets: [{ colNumber: 10, rowNumber: 10, merges: ["A1:B2"] }],
});
const model = new Model({ sheets: [{ colNumber: 10, rowNumber: 10, merges: ["A1:B2"] }] });
expect(
model.getters.zoneToXC(
model.getters.getActiveSheetId(),
Expand All @@ -439,9 +437,7 @@ describe("core", () => {
});

test("merge is considered as one single cell", () => {
const model = new Model({
sheets: [{ colNumber: 10, rowNumber: 10, merges: ["A1:B2"] }],
});
const model = new Model({ sheets: [{ colNumber: 10, rowNumber: 10, merges: ["A1:B2"] }] });
expect(
model.getters.zoneToXC(
model.getters.getActiveSheetId(),
Expand Down Expand Up @@ -710,10 +706,7 @@ describe("history", () => {
},
},
],
formats: {
"1": "#,##0",
"2": "mm/dd/yyyy",
},
formats: { "1": "#,##0", "2": "mm/dd/yyyy" },
});
activateSheet(model, sheet2Id); // evaluate Sheet2
expect(getRangeFormattedValues(model, "A1:A3", sheet1Id)).toEqual(["1,000", "", "2,000"]);
Expand Down

0 comments on commit 2e02cfc

Please sign in to comment.