feat: default to local data.json and introduce --update flag, and fix Windows test compatibility (#2896)#2952
Open
ZeYuNie wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR primarily addresses issue #2896 by changing the default loading behavior of
data.jsonto prioritize the local file bundled with the package, preventing implicit network access on execution.To preserve the ability to fetch the latest site data from the upstream repository, a new
--updateflag has been introduced. Additionally, the existing--localflag has been kept for backward compatibility but will now display aDeprecationWarning.Furthermore, while running the test suite locally on a Windows environment, I encountered and fixed two Windows-specific compatibility bugs:
test_manifest.py: Fixed aUnicodeDecodeErrorcaused by the Windows defaultGBKencoding. I explicitly addedencoding='utf-8'to the file reading operation.test_ux.pyviasherlock_interactives.py). Previously, usingsubprocess.check_outputwithshell=Trueand thepylauncher failed to resolve Windows user paths containing non-ASCII (Unicode) characters.Motivation and Context
As highlighted in #2896, implicitly fetching
data.jsonfrom GitHub causes unexpected network access, which is strictly avoided in environments like Debian. This change ensures offline execution out of the box. At the same time, the added test fixes ensure that contributors using Windows machines with non-ASCII usernames can successfully run the test suite without false-negative failures.Changes Made
--updateargument insherlock.py.api.github.comis now strictly conditioned onargs.update == True.DeprecationWarningwhenargs.localis used.encoding='utf-8'to theopen()function intests/test_manifest.py.How Has This Been Tested?
python sherlock.py user123executes immediately using the localdata.jsonwithout timing out.--updateflag and confirmed it successfully fetches the remote data and updates the local JSON file.--localflag and confirmed the deprecation warning is printed.pytestsuite locally on a Windows machine with a non-ASCII user path. Verified that all previous failing tests (test_manifest.pyandtest_ux.py) now pass successfully alongside the rest of the test suite.Types of changes
Checklist: