Skip to content

test: add edge case for config values containing '='#140

Open
YardenCuriel wants to merge 2 commits into
NVIDIA:mainfrom
YardenCuriel:test/config-edge-cases
Open

test: add edge case for config values containing '='#140
YardenCuriel wants to merge 2 commits into
NVIDIA:mainfrom
YardenCuriel:test/config-edge-cases

Conversation

@YardenCuriel

Copy link
Copy Markdown

While reading through the config parser, I noticed there's no test for values that contain '=' characters (like base64-encoded tokens). Added a test to verify this works correctly.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a regression test to ensure update_config_file correctly handles config values that contain = characters (common in base64-like tokens), protecting the config update logic from incorrectly splitting/truncating values.

Changes:

  • Add a new unit test covering updates where the value contains one or more = characters.

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

Comment thread src/config.rs Outdated
Comment thread src/config.rs Outdated
@zvonkok

zvonkok commented May 25, 2026

Copy link
Copy Markdown
Collaborator

@YardenCuriel You want to follow up with this?

  - Fix grammar: hyphenate 'base64-encoded'
  - Strengthen test assertion to verify exact file content
    instead of just substring presence
@YardenCuriel

Copy link
Copy Markdown
Author

@zvonkok sure I've addressed the Copilot review feedback

@zvonkok zvonkok added the ok-to-test Ok to test label May 28, 2026
@zvonkok

zvonkok commented May 28, 2026

Copy link
Copy Markdown
Collaborator

@YardenCuriel You have removed the actual changes and only committed the test....

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 1 out of 1 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Ok to test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants