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

CI: run integration tests on Windows #4076

Merged
merged 1 commit into from
May 5, 2022
Merged

CI: run integration tests on Windows #4076

merged 1 commit into from
May 5, 2022

Conversation

battlmonstr
Copy link
Contributor

No description provided.

@battlmonstr battlmonstr marked this pull request as draft May 4, 2022 12:28
if: runner.os != 'Windows'
run: make test-integration
- name: test-integration on Windows
if: runner.os == 'Windows'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe easier to create dedicated .yml file for win - because if in every line (but overall .yml looks simple enough).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@battlmonstr
Copy link
Contributor Author

@AndreaLanfranchi do people use this wmake standalone?

@AndreaLanfranchi
Copy link
Contributor

Yes on windows

@battlmonstr
Copy link
Contributor Author

@AndreaLanfranchi 👌 I'm gonna fix with go.exe version. Need to unpack my WIndows skillz. :)

@AndreaLanfranchi
Copy link
Contributor

If you want to keep current structure the easy way is this :

$GoCommand = Get-Command go.exe
if (!($?)) {
    Write-Host @"
    
 Error !
 Could not get the GO binary executable. Ensure GO language
 is properly installed AND its directory is properly inserted into 
 your PATH environment variable.

"@
    exit 1
} else {
    $GoVersion = & $GoCommand version
    $GoVersionValues = [regex]::Match($GoVersion, '(\d+)\.(\d+)\.(\d+)')
    if(!$GoVersionValues.Success) {
        Write-Host @"

Error !
Could not determine Go version
"@
           exit 1        
    }
    Write-Host " Found $($GoVersion)"
    $GoVersionMajor = [int]$GoVersionValues.Captures.groups[1]
    $GoVersionMinor = [int]$GoVersionValues.Captures.groups[2]
    $GoVersionValid = (($GoVersionMajor -eq 1 -and $GoVersionMinor -ge 16) -or ($GoVersionMajor -gt 1))
    if (!$GoVersionValid) {
        Write-Host @"

Error !
Insufficient Go version
Expected : 1.16 (or higher)

"@
           exit 1        

    }

}

@battlmonstr battlmonstr changed the title Wintest CI: run integration tests on WIndows May 4, 2022
@battlmonstr
Copy link
Contributor Author

@AndreaLanfranchi you are right in general. Yet in this case I believe that this approach is adequate. We do the same in the Makefile. It is a much simpler one-liner than a proper regex parsing and proper semver string comparisons.

Maybe in 5 years they release go v2 or change the format of this output string, and you'll be like "I told you so", and I'll be sprinkling ashes on my head. Bun until this day it will do thousands of successful builds, and people who maintain these files will be grateful for simplicity. I believe this is in line with the Go philosophy.

If you can't sleep with this logic, feel free to change it. I just want to make it work, because it was broken on CI for a long time (nobody noticed it, because the line was not fatal, but the wmake script was actually failing on CI all the time).

@battlmonstr battlmonstr marked this pull request as ready for review May 4, 2022 17:00
@AndreaLanfranchi
Copy link
Contributor

@battlmonstr as you prefer.
I'd generally copy and paste the suggestion snipped already prepared.
Be advised though that GO 1.9 is still around and downloadable (maybe for some reason someone can have it installed) and in such case your tests fail). https://go.dev/dl/go1.9.1.src.tar.gz

@battlmonstr
Copy link
Contributor Author

@AndreaLanfranchi , yeah, I'm sorry for that. I missed your snippet. I was a bit too fast to boot my Windows and roll my windows sleeves. Otherwise I'd use it.

About go 1.9, - good point. I need to fix this. I've checked that even "9" -ge "18" yields true, so it is doing lexicographical compare, not number compare.

@battlmonstr battlmonstr changed the title CI: run integration tests on WIndows CI: run integration tests on Windows May 4, 2022
@battlmonstr
Copy link
Contributor Author

battlmonstr commented May 4, 2022

@AndreaLanfranchi , are you on Windows? Do you know why running wmake is needed before make test?

@AndreaLanfranchi
Copy link
Contributor

are you on Windows? Do you know why running wmake is needed before make test?

Any make <target> generally does not work on Windows as make semantics are different.
That's why test is one of wmake.ps1 targets. Instead of make test use wmake.ps1 test

BTW ... as you're already there in wmake.ps1 there's double execution of tests

    Write-Host " Running tests ..."
    $env:GODEBUG="cgocheck=0"
    go test ./... -p 2 --timeout 30m
    $TestCommand = "go test ./... -p 2 --timeout 30m"
    $TestCommand += ';$?'
    $success = Invoke-Expression -Command $TestCommand

the line go test ./... -p 2 --timeout 30m should be deleted

@AndreaLanfranchi
Copy link
Contributor

Besides if you don't wmake first then no object is created so tests can run

@AskAlexSharov AskAlexSharov merged commit a0af7c0 into devel May 5, 2022
@AskAlexSharov AskAlexSharov deleted the wintest branch May 5, 2022 08:59
AlexeyAkhunov added a commit that referenced this pull request May 10, 2022
* penalize naughty peers on PoS (#4060)

* penalize naughty peers

* lint

* Small performance optimization (hash already calculated) (#4053)

* speedup logIndex test  (#4068)

* Torrent: increase network request size  (#4067)

* save

* save

* CI: run integration tests on trunk after PR is merged (#4075)

Running on PR close event tests the PR commit, not the final merged commit.
The final commit is tested by "push" event, but it appears as "skipped" in the list of devel ccmmits,
because "push" event was skipped by "if".

* Update header_algos.go (#4078)

* p2p: TestUDPv4_LookupIterator failures workaround (#4079)

--- FAIL: TestUDPv4_LookupIterator (1.36s)
155
    v4_lookup_test.go:168: handlePacket error: "unsolicited reply"
156

* CI: run integration tests on Windows (#4076)

* [erigon2] Support for binary tree commitments (#4077)

* binary tree

* Binary commitment tree

* [erigon2] Bin tree support

* Point to latest erigon-lib

Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local>

* CI: more caching (#4083)

Cache "go-build" containing cached build artifacts from the Go build system.
This saves up to:
  - 6 min on Linux (from 10 min to 4 min)
  - 3 min on macOS  (from 13 min to 10 min)
  - 7 min on Windows (from 27 min to 20 min)

Cache Windows deps (mingw, cmake).
This saves 6,5 min on Windows builds (from 20 min to 13,5 min)

* rpcdaemon: optimize tests (#4082)

* reuse the generated test blockchain across tests
* copy ChainPack to ensure test isolation

This improves the speed from 10s to 4s.

The package tests timeout can be reduced to 5s:

    go test ./cmd/rpcdaemon/commands -count 1 --timeout 5s

* Downloader atomic snapshot dir, step 1 (#4085)

* save

* save

* save

* [Docs] Add PoS downloader diagram (#4084)

* Extra comment for HeadersPOS

* Add PoS downloader diagram

* Revert "state_processor: fix ignored SkipAnalysis() result (#4046)" (#4087)

This reverts commit cf44803.

* Revert changes in decompressor (#4089)

Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local>

* Remove preverified hashes (#4088)

Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local>

* p2p: define DiscReason as uint8 (#4090)

Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local>

* remove metrics package, step 1 (#4094)

* atomic snapshot dir, step 2 (#4093)

* atomic snapshot dir

* atomic snapshot dir

* switch toml lib to release version (#4095)

* save

* save

* save (#4096)

* Snapshots: restore logInterval #4098

* torrent not found fix (#4101)

* Torrent: increase network-request size to 2Mb (#4100)

* Amend description of override.mergeForkBlock flag (#4106)

* p2p: move v4_lookup_test to integration tests (#4107)

The test is flaky when the reply timeout is too low.
Increasing the timeout makes it slow.

Move the test to the integration suite.
Having a higher timeout is fine there.

* make: wmake refactoring (#4105)

* list "all" targets explicitly
* add missing targets
* add missing build flags
* add test-integration target
* show tests output
* use wmake test on CI
* update submodules for all targets (like Makefile)
* remove unused function Test-Administrator

* Snapshots: atomic dir, step 3 (#4103)

* up linter version (#4108)

* save

* save

* save

* Rename sentry.ControlServerImpl to sentry.MultyClient and sentry.SentryServerImpl to sentry.GrpcServer #444

* Simplify header downloader (#4104)

* Simplify header downloader

* Remove VerifyQueue

* Fix

* More fixes

* Fix

* Break out of the loop

Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local>

* RPCDaemon: open snapshots on startup (because now snapshots dir is atomic), even if no Erigon available (#4110)

* save

* save

* save

* save

* save

* save

* Bumb alpha version

* Update to erigon-lib alpha

Co-authored-by: Giulio rebuffo <giulio.rebuffo@gmail.com>
Co-authored-by: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com>
Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>
Co-authored-by: battlmonstr <battlmonstr@users.noreply.github.com>
Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local>
Co-authored-by: Alex Sharp <alexsharp@Alexs-MacBook-Pro.local>
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.

3 participants