Skip to content

Commit b3c08be

Browse files
authored
Fix xeus-octave kernel startup in conformance tests (#80)
xeus-octave was crashing during startup because Octave could not find its built-in .m script files (specifically graphics_toolkit). This happened because the OCTAVE_HOME environment variable was not set when the test harness spawned the kernel process. Changes: - Set OCTAVE_HOME and OCTAVE_EXEC_PREFIX env vars in CI for xeus-octave - Increase per-test timeout to 30s for xeus-octave (Octave is slow to init) - Detect kernel process crash before attempting ZMQ connections - Capture kernel stderr for better diagnostics on startup failures - Retry kernel_info_request up to 3 times for slow-starting kernels - Increase initial kernel startup delay from 500ms to 2s
1 parent 6ee071f commit b3c08be

2 files changed

Lines changed: 113 additions & 25 deletions

File tree

.github/workflows/conformance.yml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,14 @@ jobs:
232232
if: matrix.kernel.name == 'xeus-octave'
233233
run: sudo apt-get update && sudo apt-get install -y xvfb
234234

235+
- name: Configure Octave environment
236+
if: matrix.kernel.name == 'xeus-octave'
237+
run: |
238+
# xeus-octave needs OCTAVE_HOME so Octave can find its .m scripts
239+
# (e.g., graphics_toolkit). The conda prefix contains all Octave files.
240+
echo "OCTAVE_HOME=$MAMBA_ROOT_PREFIX" >> $GITHUB_ENV
241+
echo "OCTAVE_EXEC_PREFIX=$MAMBA_ROOT_PREFIX" >> $GITHUB_ENV
242+
235243
- name: Build test suite
236244
run: cargo build --release
237245

@@ -242,6 +250,7 @@ jobs:
242250
run: |
243251
if [ "${{ matrix.kernel.name }}" = "xeus-octave" ]; then
244252
xvfb-run -a ./target/release/jupyter-kernel-test ${{ matrix.kernel.kernel-name }} \
253+
--timeout 30000 \
245254
--format json \
246255
--output ${{ matrix.kernel.name }}-report.json
247256
else
@@ -254,7 +263,7 @@ jobs:
254263
- name: Display results
255264
run: |
256265
if [ "${{ matrix.kernel.name }}" = "xeus-octave" ]; then
257-
xvfb-run -a ./target/release/jupyter-kernel-test ${{ matrix.kernel.kernel-name }} || true
266+
xvfb-run -a ./target/release/jupyter-kernel-test ${{ matrix.kernel.kernel-name }} --timeout 30000 || true
258267
else
259268
./target/release/jupyter-kernel-test ${{ matrix.kernel.kernel-name }} || true
260269
fi

src/harness.rs

Lines changed: 103 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,40 @@ impl KernelUnderTest {
110110
.map_err(|e| HarnessError::LaunchFailed(e.to_string()))?;
111111
tokio::fs::write(&connection_path, content).await?;
112112

113-
// Launch kernel process
114-
let process = kernelspec
115-
.command(&connection_path, Some(Stdio::null()), Some(Stdio::null()))?
113+
// Launch kernel process (capture stderr for diagnostics)
114+
let mut process = kernelspec
115+
.command(&connection_path, Some(Stdio::null()), Some(Stdio::piped()))?
116116
.spawn()
117117
.map_err(|e| HarnessError::LaunchFailed(e.to_string()))?;
118118

119119
// Give kernel time to start
120-
tokio::time::sleep(Duration::from_millis(500)).await;
120+
tokio::time::sleep(Duration::from_millis(2000)).await;
121+
122+
// Check if kernel process has already exited (crashed during startup)
123+
match process.try_wait() {
124+
Ok(Some(exit_status)) => {
125+
// Process has already exited - read stderr for diagnostics
126+
let mut stderr_output = String::new();
127+
if let Some(stderr) = process.stderr.take() {
128+
use tokio::io::AsyncReadExt;
129+
let mut reader = tokio::io::BufReader::new(stderr);
130+
let _ = reader.read_to_string(&mut stderr_output).await;
131+
}
132+
let msg = if stderr_output.is_empty() {
133+
format!("Kernel process exited with {} before connections could be established", exit_status)
134+
} else {
135+
format!("Kernel process exited with {} before connections could be established. Stderr:\n{}", exit_status, stderr_output)
136+
};
137+
eprintln!("{}", msg);
138+
return Err(HarnessError::LaunchFailed(msg));
139+
}
140+
Ok(None) => {
141+
// Process still running - good
142+
}
143+
Err(e) => {
144+
eprintln!("Warning: could not check kernel process status: {}", e);
145+
}
146+
}
121147

122148
// Create peer identity for shell/stdin (must share identity)
123149
let identity = peer_identity_for_session(&session_id)?;
@@ -182,29 +208,59 @@ impl KernelUnderTest {
182208
}
183209

184210
/// Fetch kernel_info and update snippets.
211+
///
212+
/// Retries the kernel_info_request up to 3 times to handle slow-starting
213+
/// kernels (e.g., xeus-octave) where the initial request may be sent before
214+
/// the kernel has bound to its ports.
185215
async fn fetch_kernel_info(&mut self) -> Result<()> {
186-
let request: JupyterMessage = KernelInfoRequest {}.into();
187-
self.shell
188-
.send(request)
189-
.await
190-
.map_err(|e| HarnessError::ProtocolError(e.to_string()))?;
216+
let max_attempts = 3;
217+
let mut last_error = None;
191218

192-
// Read reply with timeout
193-
let reply = timeout(self.test_timeout, self.shell.read())
194-
.await
195-
.map_err(|_| HarnessError::Timeout("kernel_info_reply".to_string()))?
196-
.map_err(|e| HarnessError::ProtocolError(e.to_string()))?;
219+
for attempt in 0..max_attempts {
220+
if attempt > 0 {
221+
// Wait longer between retries to give the kernel time to start
222+
tokio::time::sleep(Duration::from_secs(2)).await;
223+
}
197224

198-
if let JupyterMessageContent::KernelInfoReply(info) = reply.content {
199-
self.snippets = LanguageSnippets::for_language(&info.language_info.name);
200-
self.kernel_info = Some(*info);
201-
Ok(())
202-
} else {
203-
Err(HarnessError::ProtocolError(format!(
204-
"Expected kernel_info_reply, got {:?}",
205-
reply.content.message_type()
206-
)))
225+
let request: JupyterMessage = KernelInfoRequest {}.into();
226+
if let Err(e) = self.shell.send(request).await {
227+
last_error = Some(HarnessError::ProtocolError(e.to_string()));
228+
continue;
229+
}
230+
231+
match timeout(self.test_timeout, self.shell.read()).await {
232+
Ok(Ok(reply)) => {
233+
if let JupyterMessageContent::KernelInfoReply(info) = reply.content {
234+
self.snippets =
235+
LanguageSnippets::for_language(&info.language_info.name);
236+
self.kernel_info = Some(*info);
237+
return Ok(());
238+
} else {
239+
return Err(HarnessError::ProtocolError(format!(
240+
"Expected kernel_info_reply, got {:?}",
241+
reply.content.message_type()
242+
)));
243+
}
244+
}
245+
Ok(Err(e)) => {
246+
eprintln!(" kernel_info attempt {}: protocol error: {}", attempt + 1, e);
247+
last_error = Some(HarnessError::ProtocolError(e.to_string()));
248+
}
249+
Err(_) => {
250+
eprintln!(" kernel_info attempt {}: timeout", attempt + 1);
251+
last_error = Some(HarnessError::Timeout("kernel_info_reply".to_string()));
252+
}
253+
}
207254
}
255+
256+
// Try to capture kernel stderr for diagnostics
257+
if let Some(stderr) = self.try_read_stderr().await {
258+
if !stderr.is_empty() {
259+
eprintln!("Kernel stderr output:\n{}", stderr);
260+
}
261+
}
262+
263+
Err(last_error.unwrap_or_else(|| HarnessError::Timeout("kernel_info_reply".to_string())))
208264
}
209265

210266
/// Get kernel info.
@@ -508,6 +564,27 @@ impl KernelUnderTest {
508564
Ok(())
509565
}
510566

567+
/// Try to read any stderr output from the kernel process (for diagnostics).
568+
pub async fn try_read_stderr(&mut self) -> Option<String> {
569+
if let Some(stderr) = self.process.stderr.take() {
570+
use tokio::io::AsyncReadExt;
571+
let mut buf = Vec::new();
572+
let mut reader = tokio::io::BufReader::new(stderr);
573+
let _ = tokio::time::timeout(
574+
Duration::from_millis(100),
575+
reader.read_to_end(&mut buf),
576+
)
577+
.await;
578+
if !buf.is_empty() {
579+
Some(String::from_utf8_lossy(&buf).to_string())
580+
} else {
581+
None
582+
}
583+
} else {
584+
None
585+
}
586+
}
587+
511588
/// Shutdown the kernel cleanly.
512589
pub async fn shutdown(mut self) -> Result<()> {
513590
let request = ShutdownRequest { restart: false };
@@ -556,10 +633,12 @@ pub async fn run_conformance_suite(
556633
Ok(k) => k,
557634
Err(e) => {
558635
// Kernel failed during startup - return a partial report
636+
let error_msg = e.to_string();
637+
eprintln!("Kernel startup failed: {}", error_msg);
559638
return KernelReport::new_failed_at_startup(
560639
kernel_name,
561640
language,
562-
e.to_string(),
641+
error_msg,
563642
start.elapsed(),
564643
);
565644
}

0 commit comments

Comments
 (0)