Skip to content

Property tests#43

Closed
davidirvine wants to merge 3 commits intodevelopfrom
property-tests
Closed

Property tests#43
davidirvine wants to merge 3 commits intodevelopfrom
property-tests

Conversation

@davidirvine
Copy link
Copy Markdown
Owner

No description provided.

- Implement real-time processing validation
  * Single voice processing speed verification (sub-100ms for 1s audio)
  * Multiple voice scaling tests (1-16 voices with CPU usage monitoring)
  * Real-time constraint validation for different voice counts

- Add memory allocation stability tests
  * Long-duration processing without dynamic allocation
  * Consistent memory usage across multiple transitions
  * Amplitude and frequency bounds verification during extended processing

- Implement stress testing
  * Rapid parameter changes with fast animation speeds
  * Extreme oscillator count testing (up to 16 oscillators)
  * High-complexity scenarios with detuning and rapid transitions

- Add performance regression detection
  * Baseline performance measurement with detailed timing statistics
  * Per-sample processing time analysis (average, median, 95th percentile, max)
  * Performance consistency across different voice states

- Fix duplicate symbol issue with inline specifier for init_voice function
- Update CMakeLists.txt to include performance_tests.cpp
- All tests pass with 606,406 assertions validating performance characteristics
- Tests maintain C++14 compatibility and realistic performance expectations
- Implement frequency transition properties validation
  * Random parameter testing with 50 test cases across musical frequency range
  * Monotonicity property verification for ascending frequency transitions
  * Adaptive timeouts based on animation speed for reliable completion

- Add extreme parameter value testing
  * Very low frequencies (20-40Hz sub-bass range) stability validation
  * Very high frequencies (8-18kHz treble range) without aliasing issues
  * Extreme frequency ratio testing (32x increases/decreases across 5+ octaves)
  * Error tolerance validation within 10% for extreme transitions

- Implement Bezier curve parameter edge cases
  * Boundary value testing for control points (0.0-1.0 range)
  * Extreme control point combinations (linear, reverse, near-extremes)
  * Animation multiplier testing (0.5x to 10x speed variations)
  * Graceful handling of edge curve configurations

- Add detuning edge case validation
  * Extreme detune values (0Hz to ±100Hz) stability testing
  * Negative detuning support verification
  * Detuning behavior during frequency transitions with amplitude variation
  * Beat pattern validation with reasonable amplitude bounds

- Implement sample rate variation testing
  * Multi-rate support (8kHz to 192kHz) stability validation
  * Sample rate and frequency relationship verification
  * Proportional timing behavior across different sample rates

- Update CMakeLists.txt to include property_edge_tests.cpp
- All tests pass with 1,958,075 assertions validating comprehensive edge coverage
- Property-based testing with random parameter generation for robust validation
- Realistic bounds and tolerances for practical audio applications
Copilot AI review requested due to automatic review settings July 23, 2025 23:20
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

This PR adds comprehensive property-based testing and performance validation for the deepnote voice system. The tests focus on validating voice behavior across the entire parameter space and ensuring real-time performance requirements are met.

  • Property-based testing with random parameter generation for frequency transitions, edge cases, and extreme parameter values
  • Performance testing to validate real-time processing requirements and memory stability
  • Edge case validation for extreme frequencies, Bezier control points, detuning values, and sample rates

Reviewed Changes

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

File Description
test/property_edge_tests.cpp New comprehensive property-based tests covering frequency transitions, extreme parameters, Bezier curves, detuning, and sample rate variations
test/performance_tests.cpp New performance validation tests for real-time processing, memory stability, and stress testing
test/CMakeLists.txt Updated to include the new test files in the build
src/voice/deepnotevoice.hpp Made init_voice function inline for better test performance
Comments suppressed due to low confidence (1)

test/property_edge_tests.cpp:34

  • [nitpick] The variable name 'test' is ambiguous. Consider renaming to 'test_iteration' or 'test_case' to be more descriptive.
        for(int test = 0; test < num_tests; ++test) {

std::uniform_int_distribution<int> osc_count_dist(1, 8);
std::uniform_real_distribution<float> speed_dist(0.5f, 5.0f);

const int num_tests = 50; // Property-based testing with multiple random cases
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The magic number 50 should be defined as a named constant to improve maintainability and make it easier to adjust test coverage.

Suggested change
const int num_tests = 50; // Property-based testing with multiple random cases
const int num_tests = NUM_TESTS; // Property-based testing with multiple random cases

Copilot uses AI. Check for mistakes.
// Property: Final frequency should be close to target
float final_freq = voice.get_current_frequency().get();
float freq_error = std::abs(final_freq - target_freq);
REQUIRE(freq_error < 5.0f); // Should be within 5Hz
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The magic number 5.0f should be defined as a named constant (e.g., MAX_FREQUENCY_ERROR_HZ) to improve maintainability and make the tolerance configurable.

Suggested change
REQUIRE(freq_error < 5.0f); // Should be within 5Hz
REQUIRE(freq_error < MAX_FREQUENCY_ERROR_HZ); // Should be within 5Hz

Copilot uses AI. Check for mistakes.

// Allow some tolerance for numerical precision, but should be mostly monotonic
float non_monotonic_ratio = static_cast<float>(non_monotonic_count) / frequency_progression.size();
REQUIRE(non_monotonic_ratio < 0.1f); // Less than 10% non-monotonic steps
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The magic number 0.1f should be defined as a named constant (e.g., MAX_NON_MONOTONIC_RATIO) to improve maintainability and make the tolerance configurable.

Suggested change
REQUIRE(non_monotonic_ratio < 0.1f); // Less than 10% non-monotonic steps
REQUIRE(non_monotonic_ratio < MAX_NON_MONOTONIC_RATIO); // Less than 10% non-monotonic steps

Copilot uses AI. Check for mistakes.
INFO("Processing time for 1 second of audio: " << duration.count() << "ms");

// Should process 1 second of audio in much less than 1 second (real-time requirement)
REQUIRE(duration.count() < 100); // Less than 100ms for 1 second of audio
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The magic number 100 should be defined as a named constant (e.g., MAX_PROCESSING_TIME_MS) to improve maintainability and make the performance threshold configurable.

Suggested change
REQUIRE(duration.count() < 100); // Less than 100ms for 1 second of audio
REQUIRE(duration.count() < MAX_PROCESSING_TIME_MS); // Less than 100ms for 1 second of audio

Copilot uses AI. Check for mistakes.
Comment on lines +259 to +261
REQUIRE(avg_time_ns < sample_period_ns * 0.1); // Use only 10% of available time
REQUIRE(p95_time_ns < sample_period_ns * 0.2); // 95% of samples under 20% of time
REQUIRE(max_time_ns < sample_period_ns * 3.0); // Even worst case should be reasonable (allow for OS jitter)
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The magic numbers 0.1, 0.2, and 3.0 should be defined as named constants (e.g., CPU_USAGE_TARGET, CPU_USAGE_P95_LIMIT, CPU_USAGE_MAX_FACTOR) to improve maintainability.

Suggested change
REQUIRE(avg_time_ns < sample_period_ns * 0.1); // Use only 10% of available time
REQUIRE(p95_time_ns < sample_period_ns * 0.2); // 95% of samples under 20% of time
REQUIRE(max_time_ns < sample_period_ns * 3.0); // Even worst case should be reasonable (allow for OS jitter)
REQUIRE(avg_time_ns < sample_period_ns * CPU_USAGE_TARGET); // Use only 10% of available time
REQUIRE(p95_time_ns < sample_period_ns * CPU_USAGE_P95_LIMIT); // 95% of samples under 20% of time
REQUIRE(max_time_ns < sample_period_ns * CPU_USAGE_MAX_FACTOR); // Even worst case should be reasonable (allow for OS jitter)

Copilot uses AI. Check for mistakes.

// Should not exceed reasonable CPU usage even with many voices
// Scale expectations based on voice count
double max_cpu_percent = voice_count <= 8 ? 50.0 : 80.0; // More lenient for many voices
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The magic numbers 8, 50.0, and 80.0 should be defined as named constants to improve maintainability and make the thresholds configurable.

Suggested change
double max_cpu_percent = voice_count <= 8 ? 50.0 : 80.0; // More lenient for many voices
double max_cpu_percent = voice_count <= MAX_VOICE_COUNT_THRESHOLD
? LOW_CPU_USAGE_THRESHOLD
: HIGH_CPU_USAGE_THRESHOLD; // More lenient for many voices

Copilot uses AI. Check for mistakes.
std::mt19937 gen(rd());
}

TEST_CASE("Frequency transition properties") {

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function DOCTEST_ANON_FUNC_2 is unreachable (
DOCTEST_ANON_VAR_3
must be removed at the same time)

Copilot Autofix

AI 9 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

}
}

TEST_CASE("Extreme parameter values") {

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function DOCTEST_ANON_FUNC_21 is unreachable (
DOCTEST_ANON_VAR_22
must be removed at the same time)

Copilot Autofix

AI 9 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

}
}

TEST_CASE("Bezier curve parameter edge cases") {

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function DOCTEST_ANON_FUNC_47 is unreachable (
DOCTEST_ANON_VAR_48
must be removed at the same time)

Copilot Autofix

AI 9 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

}
}

TEST_CASE("Detuning edge cases") {

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function DOCTEST_ANON_FUNC_81 is unreachable (
DOCTEST_ANON_VAR_82
must be removed at the same time)

Copilot Autofix

AI 9 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

}
}

TEST_CASE("Sample rate variations") {

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function DOCTEST_ANON_FUNC_91 is unreachable (
DOCTEST_ANON_VAR_92
must be removed at the same time)

Copilot Autofix

AI 9 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

@davidirvine davidirvine deleted the property-tests branch July 23, 2025 23:35
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.

3 participants