On Fri, 14 Feb 2025 at 11:35, Elijah Newren [off-list ref] wrote:

quoted

quoted

Did you run any tests?  I'm not sure you maintained correctness here.

I didn't run any tests -- I wanted to, but I wasn’t sure how to do it
for this change. Since you suggested dropping this patch from the
series, I’ll do that. But for similar changes in the future, how should I go
about testing them?

As per Documentation/CodingGuidelines: "After any code change, make
sure that the entire test suite passes."  You can do that by running:
    cd t && make
(You probably want to also run that before making any changes, just to
verify that they all pass for you.  Then, if any test fails after you
make changes, you know it's because of your changes rather than
because you missed something in building or setting up the tests.)


And although it doesn't matter since we're dropping this patch, the
issue I noticed was that if there were, say, three unmerged entries
with the same path, the original code would create one entry in the
string list and modify it 3 times (each with a different ce_stage(ce).
Your modification would create three different entries (each with only
information from one stage) and drop two of them, meaning we no longer
have a single string_list_item that contains information from all 3
unmerged entries for the same path.  I'm pretty sure running the
existing tests would catch that kind of bug, which is what raised the
question.

That's the thing -- I did run make in the t/ directory, and it passed. I was
just wondering if there's any other way to test this in isolation, in case
I want to verify such changes more directly in the future.

Thanks for the clarification!
Meet