malliaridis commented on code in PR #3810:
URL: https://github.com/apache/solr/pull/3810#discussion_r2480833814
##########
solr/packaging/powershell-tests/Zk.Tests.ps1:
##########
@@ -42,6 +42,81 @@ BeforeAll {
$script:ZK_PORT = if ($env:ZK_PORT) { $env:ZK_PORT } else { "9983" }
$script:SOLR_PORT = if ($env:SOLR_PORT) { $env:SOLR_PORT } else { "8983" }
+ # Check if Solr is already running
+ $solrAlreadyRunning = $false
+ try {
+ $response = Invoke-WebRequest -Uri "http://localhost:$SOLR_PORT/solr/"
-UseBasicParsing -TimeoutSec 2 -ErrorAction SilentlyContinue
+ if ($response.StatusCode -eq 200) {
+ $solrAlreadyRunning = $true
+ Write-Host "Solr is already running on port $SOLR_PORT"
+ }
+ } catch {
+ $solrAlreadyRunning = $false
+ }
+
+ $script:SolrStartedByTests = $false
+
+ # Start Solr if it's not already running
+ if (-not $solrAlreadyRunning) {
+ Write-Host "Starting Solr in cloud mode..."
+
+ # Start Solr with embedded ZooKeeper using Start-Process to run
asynchronously
+ $startArgs = @("start", "-p", "$SOLR_PORT")
+ Write-Host "Running: $SolrCmd $($startArgs -join ' ')"
Review Comment:
An unclean exit or a service running on a chosen port are possible but
extremely rare cases the way the gradle task is written, so we could indeed
reduce the additional `-p` input. But sometimes being explicit is preferred, at
least that's what I've been told a couple of times in this project.
##########
solr/packaging/powershell-tests/Zk.Tests.ps1:
##########
@@ -42,6 +42,81 @@ BeforeAll {
$script:ZK_PORT = if ($env:ZK_PORT) { $env:ZK_PORT } else { "9983" }
$script:SOLR_PORT = if ($env:SOLR_PORT) { $env:SOLR_PORT } else { "8983" }
+ # Check if Solr is already running
+ $solrAlreadyRunning = $false
+ try {
+ $response = Invoke-WebRequest -Uri "http://localhost:$SOLR_PORT/solr/"
-UseBasicParsing -TimeoutSec 2 -ErrorAction SilentlyContinue
+ if ($response.StatusCode -eq 200) {
+ $solrAlreadyRunning = $true
+ Write-Host "Solr is already running on port $SOLR_PORT"
+ }
+ } catch {
+ $solrAlreadyRunning = $false
+ }
+
+ $script:SolrStartedByTests = $false
+
+ # Start Solr if it's not already running
+ if (-not $solrAlreadyRunning) {
+ Write-Host "Starting Solr in cloud mode..."
+
+ # Start Solr with embedded ZooKeeper using Start-Process to run
asynchronously
+ $startArgs = @("start", "-p", "$SOLR_PORT")
+ Write-Host "Running: $SolrCmd $($startArgs -join ' ')"
+
+ # Use Start-Job with cmd.exe to properly parse arguments and maintain
process hierarchy
+ $solrJob = Start-Job -ScriptBlock {
+ param($cmd, $argString)
+ cmd /c "`"$cmd`" $argString" 2>&1
+ } -ArgumentList $SolrCmd, $startArgs
+
+ # Wait a bit for Solr to start
+ Start-Sleep -Seconds 5
Review Comment:
If `bin\solr.cmd start` would properly exit after completion, it would be a
one-liner I believe. But see other comment for details.
##########
solr/packaging/powershell-tests/Zk.Tests.ps1:
##########
@@ -42,6 +42,81 @@ BeforeAll {
$script:ZK_PORT = if ($env:ZK_PORT) { $env:ZK_PORT } else { "9983" }
$script:SOLR_PORT = if ($env:SOLR_PORT) { $env:SOLR_PORT } else { "8983" }
+ # Check if Solr is already running
+ $solrAlreadyRunning = $false
+ try {
+ $response = Invoke-WebRequest -Uri "http://localhost:$SOLR_PORT/solr/"
-UseBasicParsing -TimeoutSec 2 -ErrorAction SilentlyContinue
+ if ($response.StatusCode -eq 200) {
+ $solrAlreadyRunning = $true
+ Write-Host "Solr is already running on port $SOLR_PORT"
+ }
+ } catch {
+ $solrAlreadyRunning = $false
+ }
+
+ $script:SolrStartedByTests = $false
+
+ # Start Solr if it's not already running
+ if (-not $solrAlreadyRunning) {
+ Write-Host "Starting Solr in cloud mode..."
+
+ # Start Solr with embedded ZooKeeper using Start-Process to run
asynchronously
+ $startArgs = @("start", "-p", "$SOLR_PORT")
+ Write-Host "Running: $SolrCmd $($startArgs -join ' ')"
+
+ # Use Start-Job with cmd.exe to properly parse arguments and maintain
process hierarchy
+ $solrJob = Start-Job -ScriptBlock {
+ param($cmd, $argString)
+ cmd /c "`"$cmd`" $argString" 2>&1
+ } -ArgumentList $SolrCmd, $startArgs
+
+ # Wait a bit for Solr to start
+ Start-Sleep -Seconds 5
+
+ Write-Host "Solr starting on port $SOLR_PORT (Job ID: $($solrJob.Id))"
+
+ # Check if there were any immediate errors
+ $jobOutput = Receive-Job -Job $solrJob -Keep
+ if ($jobOutput) {
+ Write-Host "Solr output: $jobOutput"
+ }
+
+ $script:SolrStartedByTests = $true
+
+ # Wait for Solr to be ready (max 60 seconds)
+ Write-Host "Waiting for Solr to be ready..."
+ $maxWaitTime = 60
+ $waitInterval = 2
+ $elapsed = 0
+ $solrReady = $false
+
+ while ($elapsed -lt $maxWaitTime) {
+ Start-Sleep -Seconds $waitInterval
+ $elapsed += $waitInterval
+
+ try {
+ $response = Invoke-WebRequest -Uri "http://localhost:$SOLR_PORT/solr/"
-UseBasicParsing -TimeoutSec 5 -ErrorAction Stop
Review Comment:
The main problem is that a simple one-liner for starting solr with
```shell
$startOutput = & $SolrCmd "start" "-p" $SOLR_PORT 2>&1
```
does not work in powershell, as the call does not exit (not sure why). So I
had to start Solr as an asynchronous job, because `Invoke Process` would also
interfere with the process hierarchy and lead to issues in `StatusTool`. So
when starting Solr as an asynchronous managed process, it will need some time
to start up. So we have to wait somehow for it to be ready.
We can get rid of the checking loop, but while testing it I had some
executions where Solr was not ready yet and pester got stuck after the first
failing test (without actually failing). So if there is a better / safer way, I
would love to use that instead.
##########
solr/packaging/powershell-tests/Zk.Tests.ps1:
##########
@@ -65,6 +140,25 @@ BeforeAll {
}
}
+AfterAll {
+ # Stop Solr only if we started it
+ if ($script:SolrStartedByTests) {
+ Write-Host "Stopping Solr..."
+ $stopArgs = @("stop", "-p", $script:SOLR_PORT)
Review Comment:
I believe we can simplify it to
```shell
AfterAll {
& $SolrCmd "stop" "--all"
}
```
but the tests won't fail if solr was not running and no Solr instance was
stopped with the command. Is that fine?
##########
solr/packaging/powershell-tests/Zk.Tests.ps1:
##########
@@ -42,6 +42,81 @@ BeforeAll {
$script:ZK_PORT = if ($env:ZK_PORT) { $env:ZK_PORT } else { "9983" }
$script:SOLR_PORT = if ($env:SOLR_PORT) { $env:SOLR_PORT } else { "8983" }
+ # Check if Solr is already running
+ $solrAlreadyRunning = $false
+ try {
+ $response = Invoke-WebRequest -Uri "http://localhost:$SOLR_PORT/solr/"
-UseBasicParsing -TimeoutSec 2 -ErrorAction SilentlyContinue
+ if ($response.StatusCode -eq 200) {
+ $solrAlreadyRunning = $true
+ Write-Host "Solr is already running on port $SOLR_PORT"
+ }
+ } catch {
+ $solrAlreadyRunning = $false
+ }
Review Comment:
Moving this out makes sense, yeah. This can be done in one go together with
the other topics I believe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]