Skip to content

Conversation

@xelArga
Copy link
Collaborator

@xelArga xelArga commented Oct 6, 2025

Description

This PR addresses the failing tests, workflows, and makefiles. There were numerous changes that were made to fix these issues.

What was changed

Linting

Most of the changes and deletions in the diff are not actual changes to the code or functionality, but most of the .c and .h code was not formatted recently with clang, and so linting all the .c and .h files is where most of that comes from. Of actual changes to code there's may 200-300 insertions/deletions.

Python Tests

  • The introduction of the new sorting folder in the query interface changed the dependency graph for the amalgamation, and the resultant header files were added.
  • The check for the directed graph headers was erroring for system header files and so a more robust check was added to pass over system headers with angle brackets <> and passes over them.
  • The directed graph was also not checking for valid files that were defined above, and so didn't match the expected headers, the validity is checked before doing the confirmation in the test

Makefile

  • The sorting method files are linked now as a bundle and passed as a SORT_OBJECTS to the query object. I also added flags for C and C++ compilation, since the tests are C++ files.

Benchmarks

  • The benchmark print statements often used unsigned long types when the data was usually either int or unsigned int 4 bytes large. I changed them to the corresponding type to remove the error messaging in the workflow runs
  • Some benchmarks would segfault, the files that did were a result of the EmbedDB state not having the active rules and number of rules initialized, the benchmarks now initialized them to NULL and 0, respectively.

Sort Test

  • The Cpp test for query sort was failing because of the same issue as above with a segfault from the state's rules not being initialized. They are now set
  • Also the column signed values weren't initialized properly at first, and just needed the correct fields so it would compile.

Sort Wrapper

  • The python test checking for cycles was failing because of a cycle with sort wrapper and advanced queries. The SortData struct was moved in it's definition from advanced queries to sort data as it seems more relevant, and is forward declared in the advanced queries, but now only the advancedQueries.c calls the sort wrapper and thus the cycle is broken as advancedQueries.h no longer calls SortWrapper.h which already calls advancedQueries.h. This fixed the test, and I think makes more sense with the sortData being moved

Workflow

  • The platform IO workflow for windows would work but the workflow file set export command for env variables to the OS default, and the set command in powershell does not hold that value for the duration of the script, meaning it would always run the example db file and not the benchmarks. It would work with export on ubuntu, but not windows. The workflow now just has an extra job, one for ubuntu and one for windows, with the env syntax being set to work on windows. It does add more jobs to the workflow and makes the file a bit longer, but I think this should be fine.

@xelArga xelArga linked an issue Oct 6, 2025 that may be closed by this pull request
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2025

Makefile Ubuntu Unit Test Results

  1 files   17 suites   0s ⏱️
113 tests 113 ✅ 0 💤 0 ❌
139 runs  139 ✅ 0 💤 0 ❌

Results for commit 175aaf0.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2025

PlatformIO Ubuntu Unit Test Results

  1 files  102 suites   18s ⏱️
113 tests 113 ✅ 0 💤 0 ❌
139 runs  139 ✅ 0 💤 0 ❌

Results for commit 175aaf0.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@rlawrenc rlawrenc left a comment

Choose a reason for hiding this comment

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

Review completed by tracing through code together

@xelArga xelArga merged commit c09dc2a into main Oct 14, 2025
18 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.

Fix the test_sort failing unit test

3 participants