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(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.

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
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.

2 participants