Merge lp:~marcustomlinson/unity-api/fix-ini-iniparser-test into lp:unity-api

Proposed by Marcus Tomlinson
Status: Merged
Approved by: Marcus Tomlinson
Approved revision: 256
Merged at revision: 255
Proposed branch: lp:~marcustomlinson/unity-api/fix-ini-iniparser-test
Merge into: lp:unity-api
Diff against target: 82 lines (+16/-8)
1 file modified
test/gtest/unity/util/IniParser/IniParser_test.cpp (+16/-8)
To merge this branch: bzr merge lp:~marcustomlinson/unity-api/fix-ini-iniparser-test
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Approve
Michi Henning (community) Needs Information
Daniel d'Andrada (community) Approve
Review via email: mp+311183@code.launchpad.net

Commit message

Don't rely on glib error message strings in IniParser_test

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:254
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/106/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3307
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3335
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3187/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3187
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3187/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/106/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Fixes the issue

review: Approve
Revision history for this message
Michi Henning (michihenning) wrote :

This changes the semantics of the existing API, I assume. Why suppress the tests to check that the contents of the error message are correct? I assume that the glib ini parser has changed what it returns? If so, we really should change the parser to emit the same error message itself, rather than just allowing anything as the exception error string.

review: Needs Fixing
Revision history for this message
Michi Henning (michihenning) wrote :

Just do be clear: does the parser emit an equivalent message that is still as informative? Or does it do something that lacks the essential information that was there previously?

review: Needs Information
255. By Marcus Tomlinson

Clean up

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

> Just do be clear: does the parser emit an equivalent message that is still as
> informative? Or does it do something that lacks the essential information that
> was there previously?

The messages are still the same, word for word. What they felt worth changing is the single quotes (e.g. group 'x') to fancy left / right double quotes (e.g. value “x”).

I don't think it's a bad idea for us to just forward on the glib error messages, but I do think it's not worth us verifying them in our tests (as things like this happen :D).

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:255
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/107/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3312
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3340
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3192/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3192
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3192/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/107/rebuild

review: Approve (continuous-integration)
256. By Marcus Tomlinson

Check for parts of error string we output

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Ok best of both: We now check for the parts of the error string we put there

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:256
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/108/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3313
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3341
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3193/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3193
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3193/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/108/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'test/gtest/unity/util/IniParser/IniParser_test.cpp'
--- test/gtest/unity/util/IniParser/IniParser_test.cpp 2016-03-29 10:08:45 +0000
+++ test/gtest/unity/util/IniParser/IniParser_test.cpp 2016-11-18 06:42:43 +0000
@@ -148,7 +148,8 @@
148 }148 }
149 catch (const LogicException& e)149 catch (const LogicException& e)
150 {150 {
151 EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));151 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get string value"));
152 EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
152 }153 }
153 try154 try
154 {155 {
@@ -156,7 +157,8 @@
156 }157 }
157 catch (const LogicException& e)158 catch (const LogicException& e)
158 {159 {
159 EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));160 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get localized string value"));
161 EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
160 }162 }
161 try163 try
162 {164 {
@@ -164,7 +166,8 @@
164 }166 }
165 catch (const LogicException& e)167 catch (const LogicException& e)
166 {168 {
167 EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));169 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer value"));
170 EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
168 }171 }
169 try172 try
170 {173 {
@@ -172,7 +175,8 @@
172 }175 }
173 catch (const LogicException& e)176 catch (const LogicException& e)
174 {177 {
175 EXPECT_NE(string::npos, string(e.what()).find("Key file contains key 'doublevalue' in group 'first' which has a value that cannot be interpreted."));178 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer value"));
179 EXPECT_NE(string::npos, string(e.what()).find("group: first"));
176 }180 }
177 try181 try
178 {182 {
@@ -180,7 +184,8 @@
180 }184 }
181 catch (const LogicException& e)185 catch (const LogicException& e)
182 {186 {
183 EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));187 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get boolean value"));
188 EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
184 }189 }
185 try190 try
186 {191 {
@@ -188,7 +193,8 @@
188 }193 }
189 catch (const LogicException& e)194 catch (const LogicException& e)
190 {195 {
191 EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a number."));196 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer array"));
197 EXPECT_NE(string::npos, string(e.what()).find("group: first"));
192 }198 }
193 try199 try
194 {200 {
@@ -196,7 +202,8 @@
196 }202 }
197 catch (const LogicException& e)203 catch (const LogicException& e)
198 {204 {
199 EXPECT_NE(string::npos, string(e.what()).find("Value '4.5' cannot be interpreted as a number."));205 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer array"));
206 EXPECT_NE(string::npos, string(e.what()).find("group: second"));
200 }207 }
201 try208 try
202 {209 {
@@ -204,7 +211,8 @@
204 }211 }
205 catch (const LogicException& e)212 catch (const LogicException& e)
206 {213 {
207 EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a boolean."));214 EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get boolean array"));
215 EXPECT_NE(string::npos, string(e.what()).find("group: first"));
208 }216 }
209}217}
210218

Subscribers

People subscribed via source and target branches

to all changes: