Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test coverage > 80% 🤞 #482

Merged
merged 2 commits into from
Mar 28, 2023
Merged

test coverage > 80% 🤞 #482

merged 2 commits into from
Mar 28, 2023

Conversation

ABevier
Copy link
Contributor

@ABevier ABevier commented Mar 27, 2023

Removed some unused functions in utils and added tests for Path to try to get to 80%

@what-the-diff
Copy link

what-the-diff bot commented Mar 27, 2023

PR Summary

  • 🗑️ Removed Array.prototype.compact_unshift
    An old and unnecessary method was removed from the code
  • Removed pivot function
    No longer needed and cleaned up from the codebase
  • 🔍 Removed attempt function
    A redundant function that was deleted
  • 🎯 Deleted tuplize helper method
    TypeScript 3+ offers native support, so this custom method is no longer needed (more info at https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-0#tuple)

@coveralls
Copy link

coveralls commented Mar 27, 2023

Coverage Status

Coverage: 79.579% (+1.9%) from 77.669% when pulling 97f1351 on coverage-to-80 into 570ddb3 on main.

@ABevier ABevier marked this pull request as ready for review March 27, 2023 22:08
@ABevier ABevier requested review from mxcl and jhheider March 27, 2023 22:08
}
yield res.value
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to re-throw if there's an error? And would this work more simply?

for await(const line of lines) {
  yield line.value
}
fd.close()

or will it actually throw?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's much better, I even did it that way in the test code. And the try/finally will allow the error to continue up the stack while correctly freeing the file resource, so catch and rethrow is no necessary.

@what-the-diff
Copy link

what-the-diff bot commented Mar 28, 2023

PR Summary

  • 🗑️ Removed Array.prototype.compact_unshift
    Removed an unnecessary extension to arrays
  • 🚀 *Added async readLines() to Path class
    Introduced a new async method for reading lines from a file
  • 🐛 Fixed readlines bug in Path class
    Resolved an issue with closing file descriptors for better resource management
  • Added tests for path module and fixed bugs found by the test suite
    Enhanced testing for the path module, and resolved discovered issues

@mxcl mxcl merged commit 4046576 into main Mar 28, 2023
@mxcl mxcl deleted the coverage-to-80 branch March 28, 2023 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants