MARP-1315 Excel Importer: Solution for adding cells with more than 255 chars

[tvtphuc-axonivy]

No description provided.

[github-actions]

Test Results

10 tests  +10   10 ✅ +10   9s ⏱️ +9s
 6 suites + 6    0 💤 ± 0 
 6 files   + 6    0 ❌ ± 0 

Results for commit a075ff8. ± Comparison against base commit 7a1ecc5.

♻️ This comment has been updated with latest results.

[ivy-rew]

change looks good;
please make the tests pass, so we can build more confidence.

[phhung-axonivy]

LGTM

[ivy-rew]

you fixed a bug; now we need a test to keep it stable.
there is a nice test exactly testing these row types and lengths. extend it to cover you scenario

[ivy-rew]

LGTM

No, I disagree ... that doesn't look good to me. @phhung-axonivy
I'd really keep the ivy market a place where professional development culture can be seen by everyone.

there are several reasons why such changes should come with a test:

• this repository has tests, it's quality should be kept and not declined 📉
• we extended a simple utility class, it's perfectly testable with simple fast executing unit tests ✔️
• having tests would it make much easier for me as reviewer, to see what you are trying to change here 🤔
• it seems as if you fixed a bug, bugs always scream for being covered with a test, unless you want the same functionality to be broken again in the near future without being noticed -> big frustration for the customer 🐞 😠
• this is product development; not project work ... products stay much longer on the market and will be used by a wider user base, so acting professional becomes way more important. 🥇

[phhung-axonivy]

Hi @ivy-rew
Thanks for your sharing.
Now, @tvtphuc-axonivy is writing the test for the fixed.
Is that right?

[tvtphuc-axonivy]

• t seems as if you fixed a bug, bugs always scream for being covered with a test, unless you want the same functionality to be broken again in the near future without being noticed -> big frustration for the customer

I am providing some UT for this issue. and I will inform you to review it.

[tvtphuc-axonivy]

Hi @ivy-rew , I just updated a unit test for the bug fix. Can you please review it to see if there are any errors in the UT?

[ivy-rew]

Wow that was real quick 🏎️
Yes, this test looks great. I like especially, that you added a new file ... rather than adding another case to the existing one.
Thanks a lot 🤝