From fa6c39617e714fb41cf3531e51e1e91eec166c4a Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Thu, 24 Jul 2025 08:51:41 +0000 Subject: [PATCH 1/3] CodeRabbit Generated Unit Tests: Expand CSVParser and BackgroundSaver unit tests for broader coverage --- test/testCSVParser.cc | 361 ++++++++++++++++++++++++++++++++++++++++++ test/testSaver.cc | 309 ++++++++++++++++++++++++++++++++++++ 2 files changed, 670 insertions(+) diff --git a/test/testCSVParser.cc b/test/testCSVParser.cc index 649eeb6fb..1731dc200 100644 --- a/test/testCSVParser.cc +++ b/test/testCSVParser.cc @@ -131,6 +131,51 @@ SUITE(CSVParser) // disable temporarily until this is fixed. TEST_FIXTURE(DataSpec,reportFromCSV) + + TEST_FIXTURE(DataSpec, reportFromCSVFileWithValidData) + { + string input="col1,col2\n"; + input += "1,2\n"; + input += "3,4\n"; + string output=""; + istringstream is(input); + ostringstream os(output); + + separator=','; + setDataArea(1,0); + numCols=2; + headerRow=0; + dimensionNames={"col1","col2"}; + dimensionCols={0}; + + reportFromCSVFile(is,os,*this,100); + string result = os.str(); + CHECK(result.find("Error") == std::string::npos); + } + + TEST_FIXTURE(DataSpec, reportFromCSVFileWithMixedErrors) + { + string input="col1,col2\n"; + input += "1,valid\n"; + input += "invalid,2\n"; + input += "3,4\n"; + string output=""; + istringstream is(input); + ostringstream os(output); + + separator=','; + setDataArea(1,0); + numCols=2; + headerRow=0; + dimensionNames={"col1","col2"}; + dimensionCols={}; + dataCols={0,1}; + + reportFromCSVFile(is,os,*this,100); + string result = os.str(); + CHECK(result.length() > 0); + } + { string input="A comment\n" ";;foobar\n" @@ -203,6 +248,46 @@ SUITE(CSVParser) }; TEST_FIXTURE(TestCSVDialog,classifyColumns) + + TEST_FIXTURE(TestCSVDialog, classifyColumnsEdgeCases) + { + string input="mixed,empty,numbers\n"; + input += "text,,123\n"; + input += "123,,456\n"; + input += ",value,789\n"; + + url=boost::filesystem::unique_path().string(); + { + ofstream of(url); + of< 0) input += ","; + input += "col" + to_string(i); + } + input += "\n"; + for(int row = 0; row < 5; ++row) { + for(int i = 0; i < 50; ++i) { + if(i > 0) input += ","; + input += to_string(row * 50 + i); + } + input += "\n"; + } + istringstream is(input); + guessFromStream(is); + CHECK_EQUAL(',', separator); + } + + TEST_FIXTURE(DataSpec, manyRows) + { + string input="col1,col2\n"; + for(int i = 0; i < 100; ++i) { + input += to_string(i) + "," + to_string(i*2) + "\n"; + } + istringstream is(input); + separator=','; + setDataArea(1,0); + numCols=2; + headerRow=0; + dimensionNames={"col1","col2"}; + dimensionCols={0}; + + VariableValue v(VariableType::parameter); + loadValueFromCSVFile(v,is,*this); + CHECK_EQUAL(2, v.rank()); + } + + TEST_FIXTURE(DataSpec, unbalancedQuotes) + { + string input="col1,col2\n"; + input += ""unclosed,normal\n"; + input += "normal,"also unclosed\n"; + istringstream is(input); + separator=','; + quote='"'; + setDataArea(1,0); + numCols=2; + headerRow=0; + dimensionNames={"col1","col2"}; + dimensionCols={0}; + + VariableValue v(VariableType::parameter); + CHECK_THROW(loadValueFromCSVFile(v,is,*this), std::exception); + } + + TEST_FIXTURE(DataSpec, inconsistentColumnCounts) + { + string input="col1,col2,col3\n"; + input += "val1,val2\n"; + input += "val3,val4,val5,val6\n"; + istringstream is(input); + separator=','; + setDataArea(1,0); + numCols=3; + headerRow=0; + dimensionNames={"col1","col2","col3"}; + dimensionCols={0}; + + VariableValue v(VariableType::parameter); + loadValueFromCSVFile(v,is,*this); + CHECK_EQUAL(2, v.rank()); + } + TEST(isNumerical) { CHECK(isNumerical("")); diff --git a/test/testSaver.cc b/test/testSaver.cc index c47b817e6..ab56d85ec 100644 --- a/test/testSaver.cc +++ b/test/testSaver.cc @@ -25,6 +25,7 @@ #undef True #include #include +#include using namespace std; using namespace classdesc; using namespace minsky; @@ -32,6 +33,9 @@ using namespace boost::filesystem; using namespace std::literals::chrono_literals; SUITE(Saver) +// Comprehensive test suite for BackgroundSaver class +// Tests cover: basic functionality, error handling, thread safety, +// performance, edge cases, and resource management { struct Fixture { @@ -90,6 +94,311 @@ SUITE(Saver) CHECK_EQUAL(os.str(), savedData); } + TEST_FIXTURE(Fixture, multipleSaves) + { + // Test multiple sequential saves to ensure data consistency + BackgroundSaver bsaver(saveFile); + schema3::Minsky m1, m2, m3; + + // Create different test data by resizing items vector + m1.items.resize(100); + m2.items.resize(200); + m3.items.resize(50); + + // Save multiple times with delays to ensure completion + bsaver.save(m1); + this_thread::sleep_for(50ms); + bsaver.save(m2); + this_thread::sleep_for(50ms); + bsaver.save(m3); + this_thread::sleep_for(100ms); + + // Verify final save is the last one (m3) + ostringstream expected; + xml_pack_t x(expected,"http://minsky.sf.net/minsky"); + xml_pack(x, "Minsky", m3); + + std::ifstream f(saveFile); + string savedData, buffer; + while (!getline(f, buffer).fail()) + savedData+=buffer; + CHECK_EQUAL(expected.str(), savedData); + } + + TEST_FIXTURE(Fixture, emptyMinsky) + { + // Test saving an empty Minsky object + BackgroundSaver bsaver(saveFile); + schema3::Minsky m; // Empty object - default constructor + bsaver.save(m); + this_thread::sleep_for(100ms); + + ostringstream expected; + xml_pack_t x(expected,"http://minsky.sf.net/minsky"); + xml_pack(x, "Minsky", m); + + std::ifstream f(saveFile); + string savedData, buffer; + while (!getline(f, buffer).fail()) + savedData+=buffer; + CHECK_EQUAL(expected.str(), savedData); + } + + TEST_FIXTURE(Fixture, largeMinsky) + { + // Test saving a large Minsky object to test performance + BackgroundSaver bsaver(saveFile); + schema3::Minsky m; + m.items.resize(10000); // Large dataset to test memory handling + + bsaver.save(m); + this_thread::sleep_for(300ms); // Allow more time for large save + + ostringstream expected; + xml_pack_t x(expected,"http://minsky.sf.net/minsky"); + xml_pack(x, "Minsky", m); + + std::ifstream f(saveFile); + string savedData, buffer; + while (!getline(f, buffer).fail()) + savedData+=buffer; + CHECK_EQUAL(expected.str(), savedData); + } + + TEST_FIXTURE(Fixture, rapidFireSaves) + { + // Test rapid successive saves to test killThread functionality + BackgroundSaver bsaver(saveFile); + schema3::Minsky m; + m.items.resize(1000); + + // Fire multiple saves in quick succession to trigger killThread + for(int i = 0; i < 5; ++i) { + bsaver.save(m); + this_thread::yield(); // Give scheduler a chance + } + + this_thread::sleep_for(200ms); + + // Verify file exists and contains valid data + CHECK(boost::filesystem::exists(saveFile)); + + ostringstream expected; + xml_pack_t x(expected,"http://minsky.sf.net/minsky"); + xml_pack(x, "Minsky", m); + + std::ifstream f(saveFile); + string savedData, buffer; + while (!getline(f, buffer).fail()) + savedData+=buffer; + CHECK_EQUAL(expected.str(), savedData); + } + + TEST_FIXTURE(Fixture, destructorSafety) + { + // Test that destructor properly calls killThread and waits for completion + schema3::Minsky m; + m.items.resize(2000); + + { + BackgroundSaver bsaver(saveFile); + bsaver.save(m); + // Destructor called here - should call killThread() and wait + } + + // File should exist and contain valid data after destructor + CHECK(boost::filesystem::exists(saveFile)); + + ostringstream expected; + xml_pack_t x(expected,"http://minsky.sf.net/minsky"); + xml_pack(x, "Minsky", m); + + std::ifstream f(saveFile); + string savedData, buffer; + while (!getline(f, buffer).fail()) + savedData+=buffer; + CHECK_EQUAL(expected.str(), savedData); + } + + TEST_FIXTURE(Fixture, errorPropagation) + { + // Test error handling through lastError mechanism + BackgroundSaver bsaver("/invalid/nonexistent/directory/file"); + schema3::Minsky m; + + // First save should fail silently (error stored in lastError) + bsaver.save(m); + this_thread::sleep_for(100ms); + + // Second save should throw the stored error + CHECK_THROW(bsaver.save(m), std::exception); + } + + TEST_FIXTURE(Fixture, fileOverwrite) + { + // Test overwriting existing files + BackgroundSaver bsaver(saveFile); + schema3::Minsky m1, m2; + m1.items.resize(100); + m2.items.resize(200); + + // First save + bsaver.save(m1); + this_thread::sleep_for(100ms); + CHECK(boost::filesystem::exists(saveFile)); + + // Second save should overwrite + bsaver.save(m2); + this_thread::sleep_for(100ms); + + // Verify final content matches m2 + ostringstream expected; + xml_pack_t x(expected,"http://minsky.sf.net/minsky"); + xml_pack(x, "Minsky", m2); + + std::ifstream f(saveFile); + string savedData, buffer; + while (!getline(f, buffer).fail()) + savedData+=buffer; + CHECK_EQUAL(expected.str(), savedData); + } + + TEST_FIXTURE(Fixture, specialCharacterPaths) + { + // Test file paths with special characters + string specialFile = saveFile + "_special-chars"; + BackgroundSaver bsaver(specialFile); + schema3::Minsky m; + + bsaver.save(m); + this_thread::sleep_for(100ms); + + CHECK(boost::filesystem::exists(specialFile)); + remove(specialFile); + } + + TEST_FIXTURE(Fixture, relativePath) + { + // Test with relative path + string relativeFile = "./test_relative_" + to_string(rand()); + BackgroundSaver bsaver(relativeFile); + schema3::Minsky m; + + bsaver.save(m); + this_thread::sleep_for(100ms); + + CHECK(boost::filesystem::exists(relativeFile)); + remove(relativeFile); + } + + TEST_FIXTURE(Fixture, performanceTest) + { + // Test performance with timing constraints + BackgroundSaver bsaver(saveFile); + schema3::Minsky m; + m.items.resize(5000); + + auto start = std::chrono::high_resolution_clock::now(); + bsaver.save(m); + this_thread::sleep_for(200ms); // Wait for completion + auto end = std::chrono::high_resolution_clock::now(); + + auto duration = std::chrono::duration_cast(end - start); + + // Verify save completed and file exists + CHECK(boost::filesystem::exists(saveFile)); + + // Basic performance check - should complete within reasonable time + CHECK(duration.count() < 3000); // 3 second timeout + } + + TEST_FIXTURE(Fixture, threadInterruption) + { + // Test killThread functionality with ongoing save + BackgroundSaver bsaver(saveFile); + schema3::Minsky m; + m.items.resize(5000); // Large enough to take some time + + bsaver.save(m); + this_thread::sleep_for(10ms); // Let save start + + // Manually call killThread (destructor will also call it) + bsaver.killThread(); + + // Should be safe to call multiple times + bsaver.killThread(); + + CHECK(true); // Test passes if no hang or crash occurs + } + + TEST_FIXTURE(Fixture, concurrentSavers) + { + // Test multiple BackgroundSavers with different files + schema3::Minsky m1, m2; + m1.items.resize(500); + m2.items.resize(1000); + + string file2 = saveFile + "_second"; + BackgroundSaver bsaver1(saveFile); + BackgroundSaver bsaver2(file2); + + // Save concurrently + bsaver1.save(m1); + bsaver2.save(m2); + + this_thread::sleep_for(200ms); + + // Both files should exist + CHECK(boost::filesystem::exists(saveFile)); + CHECK(boost::filesystem::exists(file2)); + + // Clean up second file + remove(file2); + } + + TEST_FIXTURE(Fixture, xmlValidation) + { + // Test that saved XML is well-formed and contains expected elements + BackgroundSaver bsaver(saveFile); + schema3::Minsky m; + m.items.resize(10); + + bsaver.save(m); + this_thread::sleep_for(100ms); + + std::ifstream f(saveFile); + string savedData, buffer; + while (!getline(f, buffer).fail()) + savedData+=buffer; + + // Check for basic XML structure + CHECK(savedData.find("") != string::npos); + CHECK(savedData.find("http://minsky.sf.net/minsky") != string::npos); + } + + TEST_FIXTURE(Fixture, memoryStressTest) + { + // Test with extremely large data to stress memory handling + BackgroundSaver bsaver(saveFile); + schema3::Minsky m; + + try { + m.items.resize(50000); // Very large dataset + bsaver.save(m); + this_thread::sleep_for(500ms); // Allow time for large save + + CHECK(boost::filesystem::exists(saveFile)); + } catch(const std::bad_alloc&) { + // Memory allocation failed - acceptable for stress test + CHECK(true); + } catch(...) { + // Other exceptions should be handled gracefully + CHECK(true); + } + } + TEST_FIXTURE(Fixture, badFile) { ostringstream os; From b07aa0896ba3a876ccdaa2a6350e0ab34608277c Mon Sep 17 00:00:00 2001 From: Russell Standish Date: Sat, 26 Jul 2025 10:24:58 +1000 Subject: [PATCH 2/3] Fixed compile time errors. Tests still do not pass. --- engine/CSVParser.cc | 2 ++ test/testCSVParser.cc | 33 +++++++++++++++------------------ 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/engine/CSVParser.cc b/engine/CSVParser.cc index 922f83fb1..047e4daa0 100644 --- a/engine/CSVParser.cc +++ b/engine/CSVParser.cc @@ -347,6 +347,8 @@ bool DataSpec::processChunk(std::istream& input, const TokenizerFunction& tf, si if (starts.size()-1 < firstEmpty && starts.back() 0); } + TEST_FIXTURE(DataSpec,reportFromCSV) { string input="A comment\n" ";;foobar\n" @@ -247,7 +245,6 @@ SUITE(CSVParser) void importFromCSV(const std::vector&) override {} }; - TEST_FIXTURE(TestCSVDialog,classifyColumns) TEST_FIXTURE(TestCSVDialog, classifyColumnsEdgeCases) { @@ -288,6 +285,7 @@ SUITE(CSVParser) CHECK_EQUAL(2,spec.numCols); } + TEST_FIXTURE(TestCSVDialog,classifyColumns) { string input="10,2022/10/2,hello,\n" "'5,150,000','2023/1/3','foo bar',\n" @@ -555,7 +553,6 @@ SUITE(CSVParser) } #endif - TEST_FIXTURE(DataSpec, duplicateActions) TEST_FIXTURE(DataSpec, duplicateActionsWithZeroValues) { @@ -625,6 +622,7 @@ SUITE(CSVParser) } } + TEST_FIXTURE(DataSpec, duplicateActions) { string input="A comment\n" ";;foobar\n" // horizontal dim name @@ -684,7 +682,6 @@ SUITE(CSVParser) } } - TEST_FIXTURE(DataSpec, toggleDimensions) TEST_FIXTURE(DataSpec, toggleDimensionsMultiple) { @@ -713,6 +710,7 @@ SUITE(CSVParser) CHECK_EQUAL(200, nColAxes()); } + TEST_FIXTURE(DataSpec, toggleDimensions) { toggleDimension(2); CHECK_EQUAL(1,dimensionCols.count(2)); @@ -760,16 +758,14 @@ SUITE(CSVParser) return x; } - TEST(escapeDoubledQuotes) - TEST(escapeDoubledQuotesComplexCases) { - CHECK_EQUAL("'a&'&'b'",testEscapeDoubledQuotes("'a'''b'")); + CHECK_EQUAL("'a&''b'",testEscapeDoubledQuotes("'a'''b'")); CHECK_EQUAL("'&'start&'",testEscapeDoubledQuotes("'''start''")); - CHECK_EQUAL("'end&'&'",testEscapeDoubledQuotes("'end'''")); - CHECK_EQUAL("'&'&'&'",testEscapeDoubledQuotes("'''''")); + CHECK_EQUAL("'end&''",testEscapeDoubledQuotes("'end'''")); + CHECK_EQUAL("'&'&'",testEscapeDoubledQuotes("'''''")); CHECK_EQUAL("'a,b&'c'",testEscapeDoubledQuotes("'a,b''c'")); - CHECK_EQUAL("&'&'",testEscapeDoubledQuotes("'''")); + CHECK_EQUAL("'&'",testEscapeDoubledQuotes("'''")); } TEST(escapeDoubledQuotesWithBackslashEscape) @@ -779,15 +775,16 @@ SUITE(CSVParser) spec.separator=','; spec.escape='\\'; - string test1 = ""hello""world""; + string test1 = "hello\"\"world"; escapeDoubledQuotes(test1, spec); - CHECK_EQUAL(""hello\\"world\\"", test1); + CHECK_EQUAL("hello\\\"world", test1); - string test2 = """"""; + string test2 = "\"\"\"\""; escapeDoubledQuotes(test2, spec); - CHECK_EQUAL("\\"\\"", test2); + CHECK_EQUAL("\"\\\"\"", test2); } + TEST(escapeDoubledQuotes) { CHECK_EQUAL("foo",testEscapeDoubledQuotes("foo")); CHECK_EQUAL("'foo'",testEscapeDoubledQuotes("'foo'")); @@ -921,8 +918,8 @@ SUITE(CSVParser) TEST_FIXTURE(DataSpec, unbalancedQuotes) { string input="col1,col2\n"; - input += ""unclosed,normal\n"; - input += "normal,"also unclosed\n"; + input += "\"unclosed,normal\n"; + input += "normal,\"also unclosed\n"; istringstream is(input); separator=','; quote='"'; From 50c823c63c0dd6285dabc9351b8834dc28be8c39 Mon Sep 17 00:00:00 2001 From: Russell Standish Date: Wed, 27 Aug 2025 19:31:21 +1000 Subject: [PATCH 3/3] Dynamically populate database types in database selectors. For #1880. --- RavelCAPI | 2 +- gui-js/libs/shared/src/lib/backend/minsky.ts | 1 + .../lib/connect-database/connect-database.component.ts | 6 +++--- .../src/lib/connect-database/connect-database.html | 9 +-------- .../src/lib/new-database/new-database.component.ts | 6 +++--- .../src/lib/new-database/new-database.html | 10 ++-------- 6 files changed, 11 insertions(+), 23 deletions(-) diff --git a/RavelCAPI b/RavelCAPI index d3326d2ec..593249419 160000 --- a/RavelCAPI +++ b/RavelCAPI @@ -1 +1 @@ -Subproject commit d3326d2ecc55ff4e3693b4d565c96b575c95b47b +Subproject commit 5932494197864a8f96515b7e329722d09a534f38 diff --git a/gui-js/libs/shared/src/lib/backend/minsky.ts b/gui-js/libs/shared/src/lib/backend/minsky.ts index 90cdb22d9..06bddbbca 100644 --- a/gui-js/libs/shared/src/lib/backend/minsky.ts +++ b/gui-js/libs/shared/src/lib/backend/minsky.ts @@ -2497,6 +2497,7 @@ export class ravelCAPI__Database extends CppClass { constructor(prefix: string){ super(prefix); } + async backends(): Promise {return this.$callMethod('backends');} async close(): Promise {return this.$callMethod('close');} async connect(a1: string,a2: string,a3: string): Promise {return this.$callMethod('connect',a1,a2,a3);} async connection(): Promise {return this.$callMethod('connection');} diff --git a/gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts b/gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts index 5db755a91..12d1fa31c 100644 --- a/gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts +++ b/gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts @@ -24,6 +24,7 @@ export class ConnectDatabaseComponent implements OnInit { connection: string; table=""; tables=[]; + backends=[]; ravel: Ravel; constructor( private route: ActivatedRoute, @@ -33,9 +34,8 @@ export class ConnectDatabaseComponent implements OnInit { this.ravel=new Ravel(this.electronService.minsky.canvas.item); } - ngOnInit(): void { - this.route.queryParams.subscribe((params) => { - }); + async ngOnInit() { + this.backends=await this.ravel.db.backends(); } setDbType(event) { diff --git a/gui-js/libs/ui-components/src/lib/connect-database/connect-database.html b/gui-js/libs/ui-components/src/lib/connect-database/connect-database.html index e4520a3d0..3736f6770 100644 --- a/gui-js/libs/ui-components/src/lib/connect-database/connect-database.html +++ b/gui-js/libs/ui-components/src/lib/connect-database/connect-database.html @@ -1,14 +1,7 @@
diff --git a/gui-js/libs/ui-components/src/lib/new-database/new-database.component.ts b/gui-js/libs/ui-components/src/lib/new-database/new-database.component.ts index 0adff31ca..841fc93d8 100644 --- a/gui-js/libs/ui-components/src/lib/new-database/new-database.component.ts +++ b/gui-js/libs/ui-components/src/lib/new-database/new-database.component.ts @@ -28,6 +28,7 @@ export class NewDatabaseComponent implements OnInit { connection: string; table=""; tables=[]; + backends=[]; constructor( private route: ActivatedRoute, private electronService: ElectronService, @@ -35,9 +36,8 @@ export class NewDatabaseComponent implements OnInit { ) { } - ngOnInit(): void { - this.route.queryParams.subscribe((params) => { - }); + async ngOnInit() { + this.backends=await this.electronService.minsky.databaseIngestor.db.backends(); } setDbType(event) { diff --git a/gui-js/libs/ui-components/src/lib/new-database/new-database.html b/gui-js/libs/ui-components/src/lib/new-database/new-database.html index b3398c7a9..b36c71502 100644 --- a/gui-js/libs/ui-components/src/lib/new-database/new-database.html +++ b/gui-js/libs/ui-components/src/lib/new-database/new-database.html @@ -1,14 +1,8 @@