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]

Reply via email to