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

Fix usePantry().missing() throwing PantryNotFoundError #80

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

felipecrs
Copy link
Contributor

@felipecrs felipecrs commented Jan 4, 2025

The missing() function's purpose is to avoid throwing an error when the pantry is missing. This fixes this logic and even adds a test to verify such behavior (you can verify that new test fails with the previous code).

This is first part of the fix for pkgxdev/pkgm#5.

@coveralls
Copy link

coveralls commented Jan 4, 2025

Pull Request Test Coverage Report for Build 12819552467

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 82.642%

Files with Coverage Reduction New Missed Lines %
src/hooks/usePantry.ts 4 68.71%
Totals Coverage Status
Change from base Build 12817559673: 0.02%
Covered Lines: 2453
Relevant Lines: 2903

💛 - Coveralls

@felipecrs
Copy link
Contributor Author

felipecrs commented Jan 4, 2025

@mxcl I'm pretty sure these reported coverage issues are a mistake of the checker, and not introduced by this PR.

@felipecrs felipecrs force-pushed the fix-pantry-not-found branch from dc97aaa to 3516b41 Compare January 16, 2025 23:36
@jhheider
Copy link
Contributor

jhheider commented Jan 17, 2025

yeah, it's comparing against main. no, wait, it should be. but 0.02% is pkgx-v2.

@mxcl
Copy link
Member

mxcl commented Jan 17, 2025

I do not understand the patch or the bug.

@felipecrs
Copy link
Contributor Author

felipecrs commented Jan 17, 2025

Oh. Let me explain.

@mxcl usePantry().missing() is a function whose purpose is to return true in case pantry is missing rather than throwing an error.

However, there was a code path when calling this function that could still throw an error, unexpectedly. This is the code path:

const is_cache_available = cache_available() && pantry_paths().length == 1

This code path is exercised when running usePantry() (before invoking .missing()), and only if cache_available() (thus the new test case). In such path, pantry_paths() function would be invoked and it would return an error before giving a chance to .missing().

I refactored the pantry_paths() function to no longer throw an error if the pantry is missing. After all, that's not the responsibility of this function.

The pantry_path is the same, whether it exists or not.

@felipecrs
Copy link
Contributor Author

felipecrs commented Jan 17, 2025

The other functions, the ones that will in fact access the pantry, will throw an error if the pantry is missing. Then, pkgxdev/pkgx#1067 will catch these situations and perform the pantry initialization as needed.

@mxcl
Copy link
Member

mxcl commented Jan 17, 2025

Right. I see thanks.

@mxcl mxcl merged commit b59b1a0 into pkgxdev:main Jan 17, 2025
7 of 8 checks passed
@felipecrs felipecrs deleted the fix-pantry-not-found branch January 17, 2025 14:53
@felipecrs
Copy link
Contributor Author

Thank you!

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