Template refactoring + more unittests by sanbee · Pull Request #33 · sanbee/parafeed

sanbee · 2026-04-01T15:34:23Z

The major change in this commit is refactoring of the code in clgetBaseCode.h and clgetValp() that reduces the number of templated functions to be maintained to only two.

Also added a few missing API call for dbgcl*() functions (also in clgetValp.cc)

Cleanupin the error handling framework. Added capability to set theYY_INPUT` scanner from API. However, it is not yet possible to set this multiple time dynamically at runtime.

And added some more unittests, most importantly to start testing the interactive modes as well.

reporting matching failure. This is in prep to support user-supplied functionls for more sophistacted matching and reporting.

Also setting the S->Attributes and ->CLASS bits.

and fixed the code to set the Symbol::Attributes and ::Class bits.

…rifyval_lambda

for T=int,float,bool,std::string clgetValp.cc: With the clparseVal properly templated, separate implementations for T=string or T=std::vector<string> for cl[dbg]getValp aren't necessary. This reduces the number of template functions by four. CMakeLists.txt: Removed duplication clparseVal.h tstcpp.cc, clgetValp.h, clgetBaseCode.h: Date change in Copyright notice.

This reverts commit 94c0d6e.

…work! (see commented code) SetVar.cc: Re-set S->Val in case of error during parsing/setting values.

Some cleanup + adding more unit tests.

…plate_refact

a function for scanning input stream for parsing. clgetBaseCode.h: Throw exception if the searched symbol is not found. test_parafeed_noninteractive.cpp: Not-yet-working attempts at setting shell_input function.

Nominal support for setting the input source for flex scanner (see the note below) test_parafeed_{non}interactive.cpp: Added the interactive test. Switching the input source of flex scanner does not work within a single file. All test in a single file are run in the same process in GTest. And I (SB) haven't yet figure out how to set the YY_INPUT macro to different input sources dynamically at runtime. Therefore, current the interactive and non-interactive tests are in separate file.

session get tested. Add explanatory comments.

sanbee · 2026-04-01T16:27:23Z

All iParafeedTests failed in GitHub CI. They all passed on dhruva.

sbhatnag@dhruva>ctest
Test project /home/dhruva/disk1/sanjay/Packages/parafeed/build
    Start 1: ParafeedTests
1/2 Test #1: ParafeedTests ....................   Passed    0.01 sec
    Start 2: iParafeedTests
2/2 Test #2: iParafeedTests ...................   Passed    0.01 sec

100% tests passed, 0 tests failed out of 2

Total Test time (real) =   0.02 sec

I am not sure why.

…Correctly test to be non-interactive.

agawatw · 2026-04-02T06:38:55Z

All iParafeedTests failed in GitHub CI. They all passed on dhruva.

sbhatnag@dhruva>ctest
Test project /home/dhruva/disk1/sanjay/Packages/parafeed/build
    Start 1: ParafeedTests
1/2 Test #1: ParafeedTests ....................   Passed    0.01 sec
    Start 2: iParafeedTests
2/2 Test #2: iParafeedTests ...................   Passed    0.01 sec

100% tests passed, 0 tests failed out of 2

Total Test time (real) =   0.02 sec

I am not sure why.

As Sanjay mentioned he took short cuts in the sendCmd() function with the assumption that the max_size parameter in ParafeedTest.h::test_shell_input() function is larger than the length of the input string. This can behave “fine” on some machines but fail on GitHub runners due to different compiler/flex buffer behavior. Specifically, the CI unit test reports basic_string: construction from null is not valid. If the scanner/parser state gets corrupted by bad YY_INPUT behavior, we can end up in exactly this kind of failure path.

The fix is to implement test_shell_inp as a proper chunked reader and this should eliminate the divergence of unit tests by CI and local machines.

The suggested fix is as followed.

--- a/code/test/unittest/ParafeedTest.h
+++ b/code/test/unittest/ParafeedTest.h
@@ -1,65 +1,70 @@
#include <gtest/gtest.h>
#include <cl.h>
#include <clgetValp.h>
#include <clsh.h>
#include <clinteract.h>
#include
#include
#include
+#include <algorithm>
#include <readline/readline.h>

string cmd_g;
bool firstPass=true;
+size_t cmd_pos_g=0;
class ParafeedTest : public ::testing::Test {
public:
~ParafeedTest()
{
clCleanUp();
// Reset the input mechanism for the scanner/parser
set_shell_input(backup_cl_shell_input_g);
firstPass=true;
+ cmd_pos_g=0;
}

//------------------------------------------------------------------
// Function that can supply the input to the scanner/parser from a
// string (here the global cmd_g variable).
static int test_shell_inp(char *buf, size_t max_size)
{
- int i=0;
- for (auto c : cmd_g) buf[i++]=c;
+ if (!firstPass || cmd_pos_g >= cmd_g.size()) return 0;

- i=0;
- if (firstPass) {i=cmd_g.size();firstPass=false;}
-
- return i;
+ const size_t n_to_copy = std::min(max_size, cmd_g.size() - cmd_pos_g);
+ std::memcpy(buf, cmd_g.data() + cmd_pos_g, n_to_copy);
+ cmd_pos_g += n_to_copy;
+ if (cmd_pos_g >= cmd_g.size()) firstPass=false;
+ return static_cast<int>(n_to_copy);
}

//------------------------------------------------------------------
// Save the current input function for the scanner before setting
// the scanner another function.
void sendCmd(const string& cmd)
{
cmd_g=cmd;
+ firstPass=true;
+ cmd_pos_g=0;
backup_cl_shell_input_g=get_shell_input();
set_shell_input(test_shell_inp);
}

the clgetNValBaseCode() call. See change request in PR #33. test_parafeed_interactive.cpp: Typo corrections in the comments.

(see comments for PR #33) Added std:: name space for string and vector<T> Reorganized the wrapper for clgetValp() to reduce cognitive overload for humans clgetValp.cc: Reorganized the wrapper for clgetValp() to reduce cognitive overload for humans

sanbee · 2026-04-02T14:57:46Z

As Sanjay mentioned he took short cuts in the sendCmd() function with the assumption that the max_size parameter in ParafeedTest.h::test_shell_input() function is larger than the length of the input string. This can behave “fine” on some machines but fail on GitHub runners due to different compiler/flex buffer behavior. Specifically, the CI unit test reports basic_string: construction from null is not valid. If the scanner/parser state gets corrupted by bad YY_INPUT behavior, we can end up in exactly this kind of failure path.

The fix is to implement test_shell_inp as a proper chunked reader and this should eliminate the divergence of unit tests by CI and local machines.

The suggested fix is as followed.

Can you send the output of git diff ParafeedTest.h with the above suggested changes separately? I am unable to use the pasted diff in your comment above as a patch.

agawatw · 2026-04-02T15:16:30Z

As Sanjay mentioned he took short cuts in the sendCmd() function with the assumption that the max_size parameter in ParafeedTest.h::test_shell_input() function is larger than the length of the input string. This can behave “fine” on some machines but fail on GitHub runners due to different compiler/flex buffer behavior. Specifically, the CI unit test reports basic_string: construction from null is not valid. If the scanner/parser state gets corrupted by bad YY_INPUT behavior, we can end up in exactly this kind of failure path.
The fix is to implement test_shell_inp as a proper chunked reader and this should eliminate the divergence of unit tests by CI and local machines.
The suggested fix is as followed.

Can you send the output of git diff ParafeedTest.h with the above suggested changes separately? I am unable to use the pasted diff in your comment above as a patch.

See attached.
flex.patch

test_parafeed_interactive.cpp: Removed a stray comment not applicable now.

sanbee · 2026-04-02T16:13:25Z

The patch for ParafeedTest.h did not fix the failure in iParafeedTest.

The patched however seems like a good improvement, and the ctest on dev machine passes with that patch applied also.

agawatw · 2026-04-02T21:26:16Z

The patch for ParafeedTest.h did not fix the failure in iParafeedTest.

The patched however seems like a good improvement, and the ctest on dev machine passes with that patch applied also.

After investigating this further, the issue is in the clTextColouring.cc. When TERM is unset, getenv("TERM") == NULL results in std::string construction from a null pointer. This directly matches the failing exception message (basic_string: construction from null is not valid)

Attached are the patches to fix this issue as well as a new unit test that unsets TERM and verifies clTextColouring(...) does not throw, then restores the environment variable afterward. I've tested this with env -u TERM ctest to unset TERM, mimicking what the CI runner may have.

termtest.patch

colour.patch

sanbee · 2026-04-03T00:33:58Z

Attached are the patches to fix this issue as well as a new unit test that unsets TERM and verifies clTextColouring(...) does not throw, then restores the environment variable afterward. I've tested this with env -u TERM ctest to unset TERM, mimicking what the CI runner may have.

This is impressive debugging! Now that you found it out, it also makes good sense.

Since you found it, do you want to apply the patch and push on this branch? That would be appropriate too.

…en TERM is unset (i.e. getenv("TERM") == NULL), avoiding std::string construction from a null pointer. Added a new unit test that unsets TERM and verifies clTextColouring(...) does not throw, then restores the environment variable afterward.

agawatw

All issues have been fixed.

sanbee and others added 18 commits March 22, 2026 21:16

SetVar.cc:VerfiyVal(): Use supplied functionals for matching and

d171ffd

reporting matching failure. This is in prep to support user-supplied functionls for more sophistacted matching and reporting.

clgetBaseCode.h: Added the dbg parameter for clgetNValBaseCode.

5efa90f

Also setting the S->Attributes and ->CLASS bits.

clgetBaseCode.h: Added the dbg parameter in clgetNValBaseCode(),

070c0af

and fixed the code to set the Symbol::Attributes and ::Class bits.

Merge branch 'verifyval_lambda' of github.com:sanbee/parafeed into ve…

54baa84

…rifyval_lambda

General cleanup, and further refactoring in clgetValp.cc.

8c8d6af

Minor mods and cleanup in ErrorObj.{cc,h}

94c0d6e

Revert "Minor mods and cleanup in ErrorObj.{cc,h}"

8cd5f06

This reverts commit 94c0d6e.

clgetValp.cc: Fix in clgetValpVec(), where val.push_back(d) does not …

52d3370

…work! (see commented code) SetVar.cc: Re-set S->Val in case of error during parsing/setting values.

Added the missing cldbg*getValp() functions with SMap.

46b1315

Removed unnecessary #include <shell.h>

7a18c63

Fix in CMakeLists.txt for installing GTest.

1eeae03

Some cleanup + adding more unit tests.

test_parafeed_noninterative.cpp: Enabled the WrongDataType test.

3258248

SearchSymb.cc,cl.h: Added SearchVSymb() function to the low-level API.

2c48209

Merge branch 'template_refact' of github.com:sanbee/parafeed into tem…

f53fc30

…plate_refact

cl.h, shell.l: Added get/set_shell_input() functions to get/set

f803d9b

a function for scanning input stream for parsing. clgetBaseCode.h: Throw exception if the searched symbol is not found. test_parafeed_noninteractive.cpp: Not-yet-working attempts at setting shell_input function.

Added the Interactive test where some working of the interactive

4443074

session get tested. Add explanatory comments.

sanbee requested a review from agawatw April 1, 2026 15:34

sanbee assigned agawatw Apr 1, 2026

test_parafeed_noninteractive.cpp: Fixed the ParsesClgetValpParameters…

fc5317f

…Correctly test to be non-interactive.

agawatw requested changes Apr 2, 2026

View reviewed changes

Comment thread code/clgetValp.cc Outdated

Comment thread code/clgetValp.cc Outdated

sanbw added 2 commits April 2, 2026 07:46

clgetValp.cc: The dbg parameter in clgetValVec() was not forwarded to

bbce2f1

the clgetNValBaseCode() call. See change request in PR #33. test_parafeed_interactive.cpp: Typo corrections in the comments.

ParafeedTest.h: Applied the patch suggested in PR #33.

02a4966

test_parafeed_interactive.cpp: Removed a stray comment not applicable now.

agawatw approved these changes Apr 3, 2026

View reviewed changes

agawatw merged commit efcc22d into master Apr 3, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Template refactoring + more unittests#33

Template refactoring + more unittests#33
agawatw merged 23 commits into
masterfrom
template_refact

sanbee commented Apr 1, 2026

Uh oh!

sanbee commented Apr 1, 2026

Uh oh!

Uh oh!

Uh oh!

agawatw commented Apr 2, 2026

Uh oh!

sanbee commented Apr 2, 2026

Uh oh!

agawatw commented Apr 2, 2026

Uh oh!

sanbee commented Apr 2, 2026

Uh oh!

agawatw commented Apr 2, 2026

Uh oh!

sanbee commented Apr 3, 2026

Uh oh!

agawatw left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sanbee commented Apr 1, 2026

Uh oh!

sanbee commented Apr 1, 2026

Uh oh!

Uh oh!

Uh oh!

agawatw commented Apr 2, 2026

Uh oh!

sanbee commented Apr 2, 2026

Uh oh!

agawatw commented Apr 2, 2026

Uh oh!

sanbee commented Apr 2, 2026

Uh oh!

agawatw commented Apr 2, 2026

Uh oh!

sanbee commented Apr 3, 2026

Uh oh!

agawatw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants