search

ScoopInstaller / GithubActions

Github Actions for Scoop buckets
MIT License
24 stars 22 forks source link

PR check: Fix bugs for checkver #2

Closed LazyGeniusMan closed 2 years ago

LazyGeniusMan commented 2 years ago

Try to fix https://github.com/ScoopInstaller/Versions/pull/356 and https://github.com/ScoopInstaller/Extras/pull/7586

LazyGeniusMan commented 2 years ago

Could also use something like "$manifest.Basename: \n$($object.version)" for -match regex to match manifest name and version

LazyGeniusMan commented 2 years ago

It's working, check this: https://github.com/LazyGeniusMan/Extras/pull/1#issuecomment-1000922879 https://github.com/LazyGeniusMan/Extras/actions/runs/1620077330/workflow

EDIT: Didn't pass negative testing, dont merge, I'll try something else https://github.com/LazyGeniusMan/Extras/runs/4631362476?check_suite_focus=true#step:3:285

LazyGeniusMan commented 2 years ago

Ahh, I see why it's failed, $object.version value is old version, what's the variable for new version detected by checkver?

EDIT: looks like it's intentional, it should be failed if it's not the latest version

LazyGeniusMan commented 2 years ago

Alright, it's done, sorry it's a messed up commits in PR. What I did is:

Test 1

Positive test (must passed)

LazyGeniusMan commented 2 years ago

Let me know if $outputV.Count -ge 2 is still necessary

issaclin32 commented 2 years ago

~I appreciate your effort, but I don't think removing this check entirely is a good thing. The proper way to solve the problem is to determine errors by another means (e.g. return code of a function).~

update: See comment below

Before we fix this, our maintainers can just ignore PR check fails for checkver, if we know what is going on.

LazyGeniusMan commented 2 years ago

but I don't think removing this check entirely is a good thing

Could you elaborate removing which check? I replaced it with another check which passed test all scenario that I can think of:

issaclin32 commented 2 years ago

Could you elaborate removing which check? I replaced it with another check which passed test all scenario that I can think of:

  • Correct URL and correct version found
  • Correct URL but wrong version found (not latest version)
  • Wrong URL so version not found

Sorry I was wrong. I did not think thoroughly. If Wrong URL so version not found is the case, then package: version should not appear at all. So there is no problem removing this part.

# If there are more than 2 lines and second line is not version, there is problem
$checkver = ((($outputV.Count -ge 2) -and ($outputV[1] -like "$($object.version)")))