Skip to content
New issue

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

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

Already on GitHub? # to your account

test(zstd): Fix 'zstd' extraction error in test #4651

Merged
merged 5 commits into from
Jan 13, 2022
Merged

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Jan 12, 2022

Fix Appveyor error caused by wrong zstd.exe extraction.

Description

  • Update zstd to v1.5.1 and change its 7z extraction param (This is why CI cannot find zstd)
  • Fix Mock function in zstd test cases (This is why nested zstd test fails)
  • Some unrelated tweaks
    • Install helper and set EnvVars on demand (Move from init.ps1 to test.ps1)
    • Move helper EnvVars from appveyor.yml to test.ps1
    • Change condition of Mock Get-AppFilePath to CI_WINDOWS so developers could simulate test in CI by setting $env:CI_WINDOWS = $true and modifying the helper downloading part of test.ps1 file

Motivation and Context

Relates to #4608 (comment)

How Has This Been Tested?

https://ci.appveyor.com/project/niheaven/scoop/builds/42177522/job/l8b7kyla9deyiohx

Checklist:

  • I have read the Contributing Guide.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@rashil2000 rashil2000 requested a review from chawyehsu January 12, 2022 18:15
test/bin/test.ps1 Outdated Show resolved Hide resolved
@niheaven
Copy link
Member Author

GitHub recommends _PATH suffix that point to a location on the filesystem (https://docs.github.com/en/actions/learn-github-actions/environment-variables#naming-conventions-for-environment-variables), so modify lessmsi, innounp and zstd environment variable names.

@chawyehsu
Copy link
Member

GitHub recommends _PATH suffix that point to a location on the filesystem (https://docs.github.com/en/actions/learn-github-actions/environment-variables#naming-conventions-for-environment-variables), so modify lessmsi, innounp and zstd environment variable names.

How about adding a SCOOP_ prefix for them.

@niheaven
Copy link
Member Author

How about adding a SCOOP_ prefix for them.

Just likes SCOOP_HELPERS_PATH? i.e. SCOOP_LESSMSI_PATH, SCOOP_INNOUNP_PATH and SCOOP_ZSTD_PATH?

@chawyehsu
Copy link
Member

Yes

@niheaven
Copy link
Member Author

niheaven commented Jan 13, 2022

Okay, I'll add them. Done.

@niheaven niheaven merged commit d7fb97f into develop Jan 13, 2022
@niheaven niheaven deleted the fix-test-zstd branch January 13, 2022 08:32
se35710 pushed a commit to se35710/scoop that referenced this pull request Mar 8, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants