Skip to content

Fix Promise scope issues and adopt proven integration testing patterns#41

Merged
DutchmanNL merged 5 commits intomainfrom
copilot/fix-15
Sep 19, 2025
Merged

Fix Promise scope issues and adopt proven integration testing patterns#41
DutchmanNL merged 5 commits intomainfrom
copilot/fix-15

Conversation

Copy link
Contributor

Copilot AI commented Sep 19, 2025

The integration testing examples in template.md contained JavaScript scope issues that were correctly identified in PR #12 review comments, and have been updated to adopt proven working patterns from real ioBroker adapters.

Original Issues Fixed

  1. Missing reject parameter: Promise constructors used new Promise(async (resolve) => {...}) instead of new Promise(async (resolve, reject) => {...})
  2. Incorrect error handling: Error conditions called resolve() instead of reject(err), making tests pass when they should fail
  3. Scope accessibility: The reject function wasn't available in nested async callbacks

Adopted Proven Patterns

Based on feedback from @DutchmanNL and @ticaki, the examples have been updated to match the working integration test patterns from ticaki's brightsky adapter:

Enhanced Test Structure:

// Before (basic pattern)
it('should configure and start adapter', () => new Promise(async (resolve, reject) => {
    harness.objects.getObject('system.adapter.brightsky.0', async (err, obj) => {
        if (err) {
            reject(err);
            return;
        }
        // Basic configuration and testing
    });
}));

// After (proven working pattern)
it('should configure and start adapter', function () {
    return new Promise(async (resolve, reject) => {
        try {
            harness = getHarness();
            const obj = await harness.objects.getObject('system.adapter.your-adapter.0');
            
            // Complete workflow with step-by-step logging
            console.log('✅ Step 1: Configuration written, starting adapter...');
            await harness.startAdapterAndWait();
            
            // Comprehensive state validation using dbConnection
            const stateIds = await harness.dbConnection.getStateIDs('your-adapter.0.*');
            // ... detailed validation and cleanup
        } catch (error) {
            reject(error);
        }
    });
}).timeout(40000);

Key Pattern Improvements:

  • Function Declarations: Uses function() instead of arrow functions for better timeout handling
  • Complete Workflow Testing: Shows full configuration → start → validation → cleanup workflow
  • Comprehensive State Validation: Uses harness.dbConnection.getStateIDs() to validate all created states
  • Step-by-Step Logging: Includes detailed console output for debugging
  • Proper Cleanup: Ensures await harness.stopAdapter() is called
  • Extended Timeouts: Uses 40-second timeouts to match working examples
  • Generic References: Changed from 'brightsky' to 'your-adapter' for template reusability

Updated Test Examples:

  • ✅ Complete workflow test with comprehensive state validation
  • ✅ Configuration-specific test (daily disabled scenario)
  • ✅ Missing configuration handling test with proper error detection
  • ✅ Proper async/await patterns with try/catch blocks
  • ✅ Real-world logging and debugging patterns

All automated tests continue to pass, and the JavaScript syntax validation confirms the integration testing examples now follow production-ready patterns that are proven to work in real ioBroker adapters.

Fixes #15.


💡 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.

Copilot AI and others added 2 commits September 19, 2025 02:56
Co-authored-by: DutchmanNL <7318445+DutchmanNL@users.noreply.github.com>
Co-authored-by: DutchmanNL <7318445+DutchmanNL@users.noreply.github.com>
Copilot AI changed the title [WIP] PR #16 failed Fix Promise scope issues in integration testing examples Sep 19, 2025
Copilot AI requested a review from DutchmanNL September 19, 2025 02:58
@DutchmanNL DutchmanNL requested a review from ticaki September 19, 2025 03:03
@DutchmanNL DutchmanNL marked this pull request as ready for review September 19, 2025 03:03
Copilot AI review requested due to automatic review settings September 19, 2025 03:03
Copy link
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

This PR corrects Promise handling in the integration testing examples within template.md to ensure proper error propagation and test failure behavior.

  • Add reject to Promise constructors and replace resolve on errors with reject(err)
  • Add error checks in nested state callbacks and reject on failures
  • Update CHANGELOG with a WIP entry documenting the fix

Reviewed Changes

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

File Description
template.md Fix Promise executor signatures and error handling in testing examples to prevent false-positive test passes.
CHANGELOG.md Add WIP entry describing the Promise scope/error handling fix (Fixes #15).

template.md Outdated
}
resolve();
});
}, 15000); // Allow time for API calls
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

[nitpick] 15000 is a magic number; consider extracting these timeouts to a named constant (e.g., const STATE_CHECK_DELAY_MS = 15000) and using it consistently (also at line 223 which uses 10000) to make intent clear and reduce flakiness relative to the overall test timeout.

Copilot uses AI. Check for mistakes.
@ticaki
Copy link
Collaborator

ticaki commented Sep 19, 2025

@copilot
Die Beispieleanweisungen zum Integration Testing sind teilweise inkorrekt.

Passe die Besipiele nach diesem spezifischen funktionierenden Intergrationtest an - Dabei beachten das es eine generische Anweisung sein soll.

https://github.com/ticaki/ioBroker.brightsky/blob/main/test/integration-brightsky.js

@DutchmanNL
Copy link
Contributor

@ticaki please review

@copilot Die Beispieleanweisungen zum Integration Testing sind teilweise inkorrekt.

Passe die Besipiele nach diesem spezifischen funktionierenden Intergrationtest an - Dabei beachten das es eine generische Anweisung sein soll.

https://github.com/ticaki/ioBroker.brightsky/blob/main/test/integration-brightsky.js

@copilot adopt suggestion

Co-authored-by: DutchmanNL <7318445+DutchmanNL@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 19, 2025

@ticaki please review

@copilot Die Beispieleanweisungen zum Integration Testing sind teilweise inkorrekt.
...

I've updated the integration testing examples in the template to match the proven working patterns from ticaki's brightsky adapter. The changes include:

  • Proper function() declarations instead of arrow functions
  • Complete workflow from configuration to state validation
  • Use of harness.dbConnection.getStateIDs() for comprehensive state checking
  • Step-by-step logging and proper error handling
  • Extended timeouts (40s) to match working examples
  • Generic adapter references instead of brightsky-specific ones

The examples now demonstrate real-world patterns that are proven to work in production adapters.

Commit: da27dfc

Copilot AI changed the title Fix Promise scope issues in integration testing examples Fix Promise scope issues and adopt proven integration testing patterns Sep 19, 2025
@ticaki ticaki requested a review from Copilot September 19, 2025 09:18
Copy link
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 2 out of 2 changed files in this pull request and generated 7 comments.

resolve();
console.log('🔍 Step 1: Fetching adapter object...');
const obj = await new Promise((res, rej) => {
harness.objects.getObject('system.adapter.your-adapter.0', (err, o) => {
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

Multiple references use 'your-adapter' while the main example uses 'brightsky'. All adapter references should be consistent - either use a placeholder like 'your-adapter' throughout or use 'brightsky' throughout the examples.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

ok


console.log('🔍 Step 1: Fetching adapter object...');
const obj = await new Promise((res, rej) => {
harness.objects.getObject('system.adapter.your-adapter.0', (err, o) => {
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

Multiple references use 'your-adapter' while the main example uses 'brightsky'. All adapter references should be consistent - either use a placeholder like 'your-adapter' throughout or use 'brightsky' throughout the examples.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot as @ticaki agreed with the proposal please act accordingly

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ticaki
Copy link
Collaborator

ticaki commented Sep 19, 2025

man sollte von hand noch nach brightsky.0 suchen und das durch your-adapter.0 ersetzen oder so

@DutchmanNL DutchmanNL merged commit 6aea158 into main Sep 19, 2025
2 checks passed
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.

PR #16 failed

4 participants