3887-multi-check #942

						
				@ -135,0 +163,4 @@

				            $httpBackend.expectGET('modelUrl/filter').respond(response);

				            expect(controller.rows).toEqual(data.length);

				            expect(controller.AllRowsCount).toEqual(response.length);

carlosjr commented

2022-05-09 13:34:47 +00:00

problems here:

response is an object, therefore has no length.
the tests is doing a very poor job demonstrating how the countRows logic works, would be nice to see how allRowsCount is a bigger number than the selection for example model.data.lenght < response.data.length or something similar/functional.

problems here: - response is an object, therefore has no length. - the tests is doing a very poor job demonstrating how the countRows logic works, would be nice to see how allRowsCount is a bigger number than the selection for example model.data.lenght < response.data.length or something similar/functional.

alexm marked this conversation as resolved

front/core/components/multi-check/multi-check.spec.js Outdated

						
				@ -135,0 +168,4 @@

				    });

				    describe('checkDummy()', () => {

				        it(`should set checkedDummyCount allRowsCount`, () => {

carlosjr commented

2022-05-09 13:47:16 +00:00

the following tests description leaves all the code functionality to the mainteiner's imagination, tests descriptions should explain what the test is willing to test

checkDummy() {
    if (this.checkedDummyCount)
        return this.checkedDummyCount = null;

    this.checkedDummyCount = this.allRowsCount;
}

should set "propertyName" "propertyName" vs should remove the dummy count if there was an existing one
should set "propertyName" to null vs should set the checked dummy count to all rows count if there was no count yet

Also noticed the checkDummy function not always returns a value, that seems inconsistent

the following tests description leaves all the code functionality to the mainteiner's imagination, tests descriptions should explain what the test is willing to test ``` checkDummy() { if (this.checkedDummyCount) return this.checkedDummyCount = null; this.checkedDummyCount = this.allRowsCount; } ``` should set "propertyName" "propertyName" vs should remove the dummy count if there was an existing one should set "propertyName" to null vs should set the checked dummy count to all rows count if there was no count yet Also noticed the checkDummy function not always returns a value, that seems inconsistent

alexm marked this conversation as resolved

carlosjr removed the

CR / Tests passed

label 2022-05-09 13:52:53 +00:00

alexm added 3 commits 2022-05-10 10:03:07 +00:00

fde73b35e7 Merge branch 'dev' of https://gitea.verdnatura.es/verdnatura/salix into 3887-multi-check

5919c42df2 Merge branch 'dev' of https://gitea.verdnatura.es/verdnatura/salix into 3887-multi-check

gitea/salix/pipeline/head This commit looks good Details

946f1c1cb5 refactor test

alexm added the

CR / Tests passed

label 2022-05-10 10:04:18 +00:00