fix: clear the last 4 SonarCloud findings (setup.sh install flags, pytest.raises scope) - #158
Merged
Merged
Conversation
setup.sh had never been scanned before, so newer rules surfaced on it: - pip installs use --only-binary :all: so no setup scripts run from source distributions (shell:S8541). - npm install uses --ignore-scripts (shell:S6505). Mirror the same flags in setup.bat for consistency; Sonar does not analyse .bat files, but the two scripts should not drift. Also finish python:S5778 in test_make_prediction_scaler_error: hoisting the array was not enough because the DummyModel() and BadScaler() constructors inside the pytest.raises block also count as throwing invocations. Both are now built beforehand so only make_prediction can raise.
|
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
1 participant
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
Follow-up to #157. That PR took SonarCloud from 22 open issues to 4 and turned the quality gate green, but the fresh analysis of
mainsurfaced a file that had never been scanned before (setup.sh) plus one incomplete fix. This clears the remaining 4.Changes
setup.sh— newly scanned, so current rules fired on it for the first time:shell:S8541x2 (lines 25, 26) —pip installnow passes--only-binary :all:, so pip cannot execute setup scripts from a source distribution.shell:S6505(line 32) —npm installnow passes--ignore-scripts.setup.bat— mirrors the same two flags. SonarCloud does not analyse.batfiles, but the two setup scripts should not drift apart.tests/test_predict.py—python:S5778intest_make_prediction_scaler_error. #157 hoisted the numpy array out of thepytest.raisesblock, but that was not enough: theDummyModel()andBadScaler()constructor calls inside the block also count as potentially-throwing invocations. Both are now constructed beforehand, somake_predictionis the only call that can raise.Testing
ruff check .— all checks passedpytest -q— 34 passedbash -nagainst the committedsetup.sh— parses cleanlyNote:
bash -nfails against the local working copy because the editor writes CRLF;.gitattributessets* text=auto eol=lf, so the committed blob is LF and valid. Verified viagit show HEAD:setup.sh.The setup scripts themselves were not executed end to end, since doing so would recreate the local virtualenv and reinstall the client.