search

cartesi / rollups-explorer

Cartesi Rollups Explorer Web Application
https://cartesiscan.io
Apache License 2.0
7 stars 26 forks source link

#234 Upgrade Vitest libraries to latest version #235

Closed nevendyulgerov closed 2 months ago

nevendyulgerov commented 3 months ago

I upgraded Vitest related packages to their latest versions and applied a couple of fixes for issues I noticed after the upgrade.

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rollups-explorer-base-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 3:21pm
rollups-explorer-base-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 3:21pm
rollups-explorer-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 3:21pm
rollups-explorer-optimism-mainnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 3:21pm
rollups-explorer-optimism-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 3:21pm
rollups-explorer-sepolia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 3:21pm
rollups-explorer-workshop ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 4, 2024 3:21pm
nevendyulgerov commented 2 months ago

Hi, @brunomenezes , I checked the issue with the reduced test coverage.

From the looks of it, we are covering the same files with the inclusion of .ts files that contain only type definitions - interfaces, types, enums, etc. Example.

For those files the reporter is giving a 100% coverage though, so they are not bringing down the coverage.

Between this branch and the main branch I compared the same file (TransactionProgress):

Here's a screenshot:

Screenshot 2024-09-04 at 14 55 57

The only difference I noticed was that the individual imports are not marked with ✔ anymore. The remaining content of both files is identically covered. Also, both files have the same content. So it appears that the difference in the imports coverage is causing the percentage to go down. A confirmation for this is the fact that the number of uncovered lines in TransactionProgress (43) equals the number of lines for which there is no more a ✔ symbol:

Screenshot 2024-09-04 at 17 08 16

So, it appears that the reporter is giving different results for the same file contents.

I also tried a different reporter - @vitest/coverage-istanbul but with it, the coverage fell even more - https://coveralls.io/builds/69600513. Additionally, I checked vitest's repo for any related issues. I found this one and applied the suggested solution with ignoreEmptyLines but this didn't affect the coverage result.

So, I think we can either ignore the failed coveralls check for this PR and merge it like this with this new test coverage result or I can add more unit tests to prevent the decrease of test coverage.

brunomenezes commented 2 months ago

Hi, @brunomenezes , I checked the issue with the reduced test coverage.

@nevendyulgerov, that is alright. As we discussed, they changed a bit about how mapping works. That becomes the new threshold, and we will work from here.