Skip to content

fix: windows cases failed issues#35014

Merged
guanshengliang merged 2 commits intomainfrom
fix/windowsCiIssues
Mar 31, 2026
Merged

fix: windows cases failed issues#35014
guanshengliang merged 2 commits intomainfrom
fix/windowsCiIssues

Conversation

@dapan1121
Copy link
Copy Markdown
Contributor

…y three cases failed on windows issues

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

@dapan1121 dapan1121 requested review from a team, guanshengliang and zitsen as code owners March 31, 2026 07:55
Copilot AI review requested due to automatic review settings March 31, 2026 07:55
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adjusts Windows timezone offset calculations to align with POSIX conventions and improves carriage return handling in the shell engine. A critical issue was identified in the test framework where a new return statement prevents the server's running state from being updated, which would cause subsequent operations like stopping the node to fail.

Comment thread test/new_test_framework/utils/server/dnode.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes several Windows-specific failures by normalizing CRLF handling, adjusting Windows process start flow in tests, and aligning timezone offset sign conventions.

Changes:

  • Strip \r cleanly when sourcing shell SQL files on Windows.
  • Prevent startWithoutSleep() from falling through into non-Windows command execution on Windows.
  • Negate Windows timezone offsets to match POSIX tm_gmtoff (east-positive) expectations in timestamp formatting/parsing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
tools/shell/src/shellEngine.c Changes CR handling when reading sourced SQL lines (CRLF).
test/new_test_framework/utils/server/dnode.py Adjusts Windows start path in startWithoutSleep() to avoid later use of cmd.
source/common/src/ttime.c Fixes Windows timezone offset sign to match POSIX conventions; adds explanatory comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/shell/src/shellEngine.c
Comment thread test/new_test_framework/utils/server/dnode.py
Comment thread source/common/src/ttime.c Outdated
Comment thread source/common/src/ttime.c Outdated
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 31, 2026 08:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/new_test_framework/utils/server/dnode.py
Comment thread test/new_test_framework/utils/server/dnode.py
@guanshengliang guanshengliang merged commit f871caf into main Mar 31, 2026
19 of 21 checks passed
jiajingbin pushed a commit that referenced this pull request Apr 8, 2026
wangmm0220 pushed a commit that referenced this pull request Apr 14, 2026
@dapan1121 dapan1121 deleted the fix/windowsCiIssues branch April 30, 2026 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants