Remove or replace `compare_err_stream_substring` when an empty string is being compared by mitchute · Pull Request #11648 · NatLabRockies/EnergyPlus

mitchute · 2026-06-18T21:01:41Z

Pull request overview

The behavior of compare_err_stream_substring appears to have been misunderstood, (propagated via copy/paste,) and possibly allowed errors to pass by uncaught. For std::string::find(""), C++ considers the empty string found at position 0 for any string, including another empty string. See the MWE, here.

This makes usages like compare_err_stream_substring("", true) ineffective at doing anything other than clearing the err stream. If the intent is really to expect an empty err stream, we should be using compare_err_stream instead.

This PR replaces those usages, and adds guards to protect against further misuse.

Description of the purpose of this PR

Pull Request Author

Reviewer

…being compared

…ssed when searching for substrings

mitchute · 2026-06-18T21:04:56Z

Locally, I'm still seeing the following as a result of these changes:

1649: [ RUN      ] EnergyPlusFixture.OutdoorAirUnit_AutoSize
1649: /home/mitchute/Projects/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.cc:223: Failure
1649: Expected equality of these values:
1649:   expected_string
1649:     Which is: ""
1649:   stream_str
1649:     Which is: "   ** Warning ** SizeDXCoil Coil:Cooling:DX:SingleSpeed ACDXCOIL 1\n   **   ~~~   ** ...Gross Rated Total Cooling Capacity [W] will be limited by the maximum rated volume flow per rated total capacity ratio.\n   **   ~~~   ** ...DX coil volume flow rate [m3/s] = 0.500000\n   **   ~~~   ** ...Requested capacity [W] = 75.000\n   **   ~~~   ** ...Requested flow/capacity ratio [m3/s/W] = 0.00666667\n   **   ~~~   ** ...Maximum flow/capacity ratio [m3/s/W] = 6.04100E-05\n   **   ~~~   ** ...Adjusted capacity [W] = 8276.775\n   ** Warning ** SizeHeatingCoil: : Potential issue with equipment sizing for Coil:Heating:Electric ZONE1OAUHEATINGCOIL\n   **   ~~~   ** ...Rated Total Heating Capacity = 0.00 [W]\n   **   ~~~   ** ...Capacity used to size child component set to 0 [W]\n"
1649: With diff:
1649: @@ -1,1 +1,10 @@
1649: -""
1649: +   ** Warning ** SizeDXCoil Coil:Cooling:DX:SingleSpeed ACDXCOIL 1
1649: +   **   ~~~   ** ...Gross Rated Total Cooling Capacity [W] will be limited by the maximum rated volume flow per rated total capacity ratio.
1649: +   **   ~~~   ** ...DX coil volume flow rate [m3/s] = 0.500000
1649: +   **   ~~~   ** ...Requested capacity [W] = 75.000
1649: +   **   ~~~   ** ...Requested flow/capacity ratio [m3/s/W] = 0.00666667
1649: +   **   ~~~   ** ...Maximum flow/capacity ratio [m3/s/W] = 6.04100E-05
1649: +   **   ~~~   ** ...Adjusted capacity [W] = 8276.775
1649: +   ** Warning ** SizeHeatingCoil: : Potential issue with equipment sizing for Coil:Heating:Electric ZONE1OAUHEATINGCOIL
1649: +   **   ~~~   ** ...Rated Total Heating Capacity = 0.00 [W]
1649: +   **   ~~~   ** ...Capacity used to size child component set to 0 [W]\n
1649: 
1/1 Test #1649: EnergyPlusFixture.OutdoorAirUnit_AutoSize ...SIGTRAP***Exception:   1.23 sec

@rraustad do you have thoughts on this one?

rraustad · 2026-06-19T00:14:03Z

@mitchute changing from what you describe as a useless compare_err_stream_substring("", true); (I wasn't sure what that was doing anyway) to an actual check of the error string using compare_err_stream("", true); I would expect that there would be phantom errors showing up here and there in unit tests. I guess I would delete that error check or change "" to something more informative (e.g., "Gross Rated Total Cooling Capacity [W] will be limited by the maximum rated volume flow per rated total capacity ratio").

rraustad · 2026-06-19T00:16:40Z

+    if (search_string.empty()) {
+        ADD_FAILURE() << "compare_eio_stream_substring cannot search for an empty string; use compare_eso_stream or has_eso_output instead.";
+        return false;
+    }


I do like this though! Now we can't be lazy.

mitchute · 2026-06-19T02:36:08Z

@rraustad the error I show above is a phantom error that was being thrown and hidden, but is now appearing. It's occurring due to this line being removed:

https://github.com/NatLabRockies/EnergyPlus/pull/11648/changes#diff-5df7d341b7cc0593a8f16b92812f16290e3e83e9961a3030960cae9b8fa35582L364

compare_err_stream_substring("", true) was essentially not checking anything, resetting the err stream, and then moving on.

mitchute

Ready for review

mitchute · 2026-06-22T21:12:57Z

+    if (search_string.empty()) {
+        ADD_FAILURE() << "compare_eio_stream_substring cannot search for an empty string; use compare_eso_stream or has_eso_output instead.";
+        return false;
+    }


Requires the compare stream functions to need a non-empty string to compare against.

mitchute · 2026-06-22T21:15:05Z


    ASSERT_TRUE(process_idf(idf_objects));
-    compare_err_stream_substring("", true); // clear idf errors
+    compare_err_stream("", true); // clear idf errors


Converts to compare_err_stream so we don't accidentally mask actual errors.

mitchute · 2026-06-22T21:16:14Z

+    std::string const sizing_error_string = delimited_string({
+        "   ** Warning ** SizeDXCoil Coil:Cooling:DX:SingleSpeed ACDXCOIL 1",
+        "   **   ~~~   ** ...Gross Rated Total Cooling Capacity [W] will be limited by the maximum rated volume flow per rated total capacity ratio.",
+        "   **   ~~~   ** ...DX coil volume flow rate [m3/s] = 0.500000",
+        "   **   ~~~   ** ...Requested capacity [W] = 75.000",
+        "   **   ~~~   ** ...Requested flow/capacity ratio [m3/s/W] = 0.00666667",
+        "   **   ~~~   ** ...Maximum flow/capacity ratio [m3/s/W] = 6.04100E-05",
+        "   **   ~~~   ** ...Adjusted capacity [W] = 8276.775",
+        "   ** Warning ** SizeHeatingCoil: : Potential issue with equipment sizing for Coil:Heating:Electric ZONE1OAUHEATINGCOIL",
+        "   **   ~~~   ** ...Rated Total Heating Capacity = 0.00 [W]",
+        "   **   ~~~   ** ...Capacity used to size child component set to 0 [W]",
+    });
+    EXPECT_TRUE(compare_err_stream(sizing_error_string, true));


This was being suppressed, so I'm adding the message here. If there's a separate issue with the model, that should probably be handled separately.

mitchute · 2026-06-22T21:16:53Z


    ASSERT_TRUE(process_idf(idf_objects));
-    compare_eio_stream_substring("", true);
+    compare_err_stream("", true);


I'm guessing the err stream was what was intended.

mitchute · 2026-07-06T22:27:43Z

All good here. This one is next.

mitchute added 2 commits June 18, 2026 14:26

remove/replace compare_err_stream_substring when an empty string was …

79752a8

…being compared

patch a few misapplied usages, ensure empty search strings are not pa…

a1b9d7e

…ssed when searching for substrings

mitchute added the Defect Includes code to repair a defect in EnergyPlus label Jun 18, 2026

rraustad reviewed Jun 19, 2026

View reviewed changes

mitchute added 3 commits June 22, 2026 13:22

Merge branch 'develop' into fix-compare-err-substring

1e3edf5

add in phantom warning text

6e48e5c

Merge branch 'develop' into fix-compare-err-substring

563c168

mitchute commented Jun 22, 2026

View reviewed changes

mitchute merged commit 9ba7b15 into develop Jul 6, 2026
11 checks passed

mitchute deleted the fix-compare-err-substring branch July 6, 2026 23:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove or replace `compare_err_stream_substring` when an empty string is being compared#11648

Remove or replace `compare_err_stream_substring` when an empty string is being compared#11648
mitchute merged 5 commits into
developfrom
fix-compare-err-substring

mitchute commented Jun 18, 2026

Uh oh!

mitchute commented Jun 18, 2026

Uh oh!

rraustad commented Jun 19, 2026

Uh oh!

rraustad Jun 19, 2026

Uh oh!

mitchute commented Jun 19, 2026 •

edited

Loading

Uh oh!

mitchute left a comment

Uh oh!

mitchute Jun 22, 2026

Uh oh!

mitchute Jun 22, 2026

Uh oh!

mitchute Jun 22, 2026

Uh oh!

mitchute Jun 22, 2026

Uh oh!

mitchute commented Jul 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mitchute commented Jun 18, 2026

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

mitchute commented Jun 18, 2026

Uh oh!

rraustad commented Jun 19, 2026

Uh oh!

rraustad Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

mitchute commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mitchute left a comment

Choose a reason for hiding this comment

Uh oh!

mitchute Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

mitchute Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

mitchute Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

mitchute Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

mitchute commented Jul 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mitchute commented Jun 19, 2026 •

edited

Loading