Review and merge PR #79

[sulenchy]

Discuss this ticket with your team. Although it is a QA ticket, the QA engineer may still request some changes, so it is necessary to assign each PR to a developer. The developer will handle the requested changes.

PR #79

[FredrikMohag]

Hey, I noticed the test files (createEle.test.js, ifValidNumber.test.js, and numberToLetter.test.js) should be in src/js/test/helpers/, but I’m having trouble seeing them in my local environment. Could you check if they’ve been committed and pushed properly? If they’re missing, we might need to add them again. @BergitTveit

[BergitTveit]

Hey, I will look into this and get back to you @FredrikMohag ☺️

[BergitTveit]

Hi @FredrikMohag are you sure that you pulled correct branch? On my side everything seem to be fetched correctly. I have just checked out branch feature/vitest-helpers-tests and all files are there in place. Is that what you are wandering about? However the test that are in PR are failing, but I am not sure if that is relevant for now?

 FAIL  src/js/test/helpers/ifValidNumber.test.js > ifValidNumber function > should round down non-integer values if they are valid
AssertionError: expected undefined to deeply equal [ 5, 9 ]

- Expected:
Array [
  5,
  9,
]

+ Received:
undefined

 ❯ src/js/test/helpers/ifValidNumber.test.js:35:24
     33|     it('should round down non-integer values if they are valid', () => {
     34|         const result = ifValidNumber(5.9, 9.7);
     35|         expect(result).toEqual([5, 9]);
       |                        ^
     36|     });
     37|

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯

 FAIL  src/js/test/helpers/numberToLetter.test.js > numberToLetter function > handles large numbers
AssertionError: expected 'undefinedA' to be 'AAA' // Object.is equality

- Expected
+ Received

- AAA
+ undefinedA

 ❯ src/js/test/helpers/numberToLetter.test.js:23:37
     21|     it('handles large numbers', () => {
     22|         expect(numberToLetter(701)).toBe('ZZ');
     23|         expect(numberToLetter(702)).toBe('AAA');
       |                                     ^
     24|     });
     25|

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/2]⎯

 Test Files  2 failed | 2 passed (4)
      Tests  2 failed | 13 passed (15)
   Start at  21:21:45
   Duration  10.71s (transform 92ms, setup 0ms, collect 259ms, tests 18ms, environment 12.92s, prepare 28.33s)

[FredrikMohag]

Oops @BergitTveit , my mistake! I have a suggestion for improvement: It might be better to throw errors in ifValidNumber instead of using console.error. This approach would provide more consistent error handling throughout the code and make it easier to test and manage invalid inputs.

[FredrikMohag]

@BergitTveit I'm able to see the test files now :)