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

Proposed by Marcus Tomlinson on 2016-11-17
Status: Merged
Approved by: Marcus Tomlinson on 2016-11-18
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 on 2016-11-18
Michi Henning (community) Needs Information on 2016-11-17
Daniel d'Andrada (community) 2016-11-17 Approve on 2016-11-17
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.
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)
Daniel d'Andrada (dandrader) wrote :

Fixes the issue

review: Approve
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
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 on 2016-11-18

Clean up

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).

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 on 2016-11-18

Check for parts of error string we output

Marcus Tomlinson (marcustomlinson) wrote :

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

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
1=== modified file 'test/gtest/unity/util/IniParser/IniParser_test.cpp'
2--- test/gtest/unity/util/IniParser/IniParser_test.cpp 2016-03-29 10:08:45 +0000
3+++ test/gtest/unity/util/IniParser/IniParser_test.cpp 2016-11-18 06:42:43 +0000
4@@ -148,7 +148,8 @@
5 }
6 catch (const LogicException& e)
7 {
8- EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
9+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get string value"));
10+ EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
11 }
12 try
13 {
14@@ -156,7 +157,8 @@
15 }
16 catch (const LogicException& e)
17 {
18- EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
19+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get localized string value"));
20+ EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
21 }
22 try
23 {
24@@ -164,7 +166,8 @@
25 }
26 catch (const LogicException& e)
27 {
28- EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
29+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer value"));
30+ EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
31 }
32 try
33 {
34@@ -172,7 +175,8 @@
35 }
36 catch (const LogicException& e)
37 {
38- EXPECT_NE(string::npos, string(e.what()).find("Key file contains key 'doublevalue' in group 'first' which has a value that cannot be interpreted."));
39+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer value"));
40+ EXPECT_NE(string::npos, string(e.what()).find("group: first"));
41 }
42 try
43 {
44@@ -180,7 +184,8 @@
45 }
46 catch (const LogicException& e)
47 {
48- EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
49+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get boolean value"));
50+ EXPECT_NE(string::npos, string(e.what()).find("group: foo"));
51 }
52 try
53 {
54@@ -188,7 +193,8 @@
55 }
56 catch (const LogicException& e)
57 {
58- EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a number."));
59+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer array"));
60+ EXPECT_NE(string::npos, string(e.what()).find("group: first"));
61 }
62 try
63 {
64@@ -196,7 +202,8 @@
65 }
66 catch (const LogicException& e)
67 {
68- EXPECT_NE(string::npos, string(e.what()).find("Value '4.5' cannot be interpreted as a number."));
69+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get integer array"));
70+ EXPECT_NE(string::npos, string(e.what()).find("group: second"));
71 }
72 try
73 {
74@@ -204,7 +211,8 @@
75 }
76 catch (const LogicException& e)
77 {
78- EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a boolean."));
79+ EXPECT_NE(string::npos, string(e.what()).find("unity::LogicException: Could not get boolean array"));
80+ EXPECT_NE(string::npos, string(e.what()).find("group: first"));
81 }
82 }
83

Subscribers

People subscribed via source and target branches

to all changes: