Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 2, 2025

  • Understand the current watch test flow and how nodemon handles file changes
  • Add a new test case that triggers a file change during watch mode
  • Implement logic to modify adapter main file (touch or actual change)
  • Add validation to check for adapter termination logs
  • Add validation to check for adapter restart logs
  • Verify test works for JS adapters
  • Verify test works for TS adapters (basic implementation complete)
  • Run all tests to ensure no regressions
  • Address code review feedback:
    • Moved assert require to top of file
    • Optimized restart detection pattern (removed redundant check)
    • Changed validation to use === 2 instead of >= 2 for exact counts
    • Refactored to eliminate code duplication between runCommandWithSignal and runCommandWithFileChange

Summary:
Enhanced watch command test suite with proper validation of adapter restart behavior. Refactored test utilities to use a common runCommandWithTimeout function that eliminates code duplication while maintaining clean separation of concerns through callback-based customization.

Original prompt

This section details on the original issue you should resolve

<issue_title>Enhance "watch" command testing to verify that adapter gets restarted</issue_title>
<issue_description>In the watch mode of the dev-server the adapter files (e.g main.js/ts) is monitored for changes. If the file is changed ( maybe even a touch is enough?) and then the adapter to test is restarted.

Enhance the "watch" testing to include and validate this case. The log should show lines that the adapter is "terminating" and then started again.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #525

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Enhance "watch" command testing to verify that adapter gets restarted Add adapter restart validation to watch command tests Oct 2, 2025
@Copilot Copilot AI requested a review from Apollon77 October 2, 2025 22:37
Copilot finished work on behalf of Apollon77 October 2, 2025 22:37

// Should see nodemon restart messages
assert.ok(
output.includes('restarting') || output.includes('restart'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

@copilot the second pattern is included in the first one. Please optimize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Optimized to check only for 'restarting' (commit: {{COMMIT_HASH}})

* Validate that adapter restart occurred in watch mode
*/
function validateWatchRestartOutput(output, adapterPrefix) {
const assert = require('node:assert');
Copy link
Collaborator

Choose a reason for hiding this comment

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

@copilot do Not use local requires make them on start of file if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved assert require to top of file (commit: {{COMMIT_HASH}})

// Should see the test variable deletion message at least twice
const testVarMatches = output.match(new RegExp(`state ${adapterPrefix}\\.0\\.testVariable deleted`, 'g'));
assert.ok(
testVarMatches && testVarMatches.length >= 2,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@copilot why more then twice? Test as specific as possible. Potentially verify with multiple runs what can happen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to use === 2 for exact count validation (commit: {{COMMIT_HASH}})

/**
* Run watch command with file change trigger to test adapter restart
*/
function runCommandWithFileChange(command, args, options = {}) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@copilot seems the new function is like the runCommandWithSignal and just adds a new file change logic but now duplicates a lot code. Refactor please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored to use common runCommandWithTimeout function, eliminating code duplication (commit: {{COMMIT_HASH}})

Co-authored-by: Apollon77 <11976694+Apollon77@users.noreply.github.com>
@Copilot Copilot AI requested a review from Apollon77 October 4, 2025 08:33
Copilot finished work on behalf of Apollon77 October 4, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance "watch" command testing to verify that adapter gets restarted
2 participants