Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch 'Ice/optional' Tests to use Optional Structs Instead of Optional Classes #2094

Merged
merged 6 commits into from
May 1, 2024
Merged

Switch 'Ice/optional' Tests to use Optional Structs Instead of Optional Classes #2094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7d3c7c1
Switch some tests to use optional structs instead of optional classes.
InsertCreativityHere Apr 30, 2024
9947059
First round of fixes to other languages
InsertCreativityHere Apr 30, 2024
356a13e
Hopefully the last of them.
InsertCreativityHere Apr 30, 2024
11ae20f
Remove send/return optional test functions.
InsertCreativityHere May 1, 2024
1468b4c
We were reusing a variable later on in a test...
InsertCreativityHere May 1, 2024
8725b8d
More variable re-use.
InsertCreativityHere May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 23 additions & 35 deletions cpp/test/Ice/optional/AllTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,11 @@ class FObjectReader : public Ice::Value
_f = make_shared<F>();
in->startValue();
in->startSlice();
// Don't read af on purpose
// in.read(1, _f->af);
// Don't read fsf on purpose
// in.read(1, _f->fsf);
in->endSlice();
in->startSlice();
in->read(_f->ae);
in->read(_f->fse);
in->endSlice();
in->endValue();
}
Expand Down Expand Up @@ -644,28 +644,16 @@ allTests(Test::TestHelper* helper, bool)
factory->setEnabled(false);
}

//
// Use the 1.0 encoding with operations whose only class parameters are optional.
//
optional<OneOptionalPtr> oo(make_shared<OneOptional>(53));
initial->sendOptionalClass(true, oo);
initial->ice_encodingVersion(Ice::Encoding_1_0)->sendOptionalClass(true, oo);

initial->returnOptionalClass(true, oo);
test(oo);
initial->ice_encodingVersion(Ice::Encoding_1_0)->returnOptionalClass(true, oo);
test(!oo);

GPtr g = make_shared<G>();
g->gg1Opt = make_shared<G1>("gg1Opt");
g->gg2 = make_shared<G2>(10);
g->gg2Opt = make_shared<G2>(20);
g->gg1 = make_shared<G1>("gg1");
g->gg1Opt = G1{"gg1Opt"};
g->gg2 = G2{10};
g->gg2Opt = G2{20};
g->gg1 = G1{"gg1"};
GPtr r = initial->opG(g);
test("gg1Opt" == r->gg1Opt.value()->a);
test(10 == r->gg2->a);
test(20 == r->gg2Opt.value()->a);
test("gg1" == r->gg1->a);
test("gg1Opt" == r->gg1Opt.value().a);
test(10 == r->gg2.a);
test(20 == r->gg2Opt.value().a);
test("gg1" == r->gg1.a);

initial->opVoid();

Expand Down Expand Up @@ -765,15 +753,15 @@ allTests(Test::TestHelper* helper, bool)

cout << "ok" << endl;

cout << "testing marshaling of objects with optional objects..." << flush;
cout << "testing marshaling of objects with optional members..." << flush;
{
FPtr f = make_shared<F>();

f->af = make_shared<A>();
f->ae = *f->af;
f->fsf = FixedStruct{56};
f->fse = *f->fsf;

FPtr rf = dynamic_pointer_cast<F>(initial->pingPong(f));
test(rf->ae == *rf->af);
test(rf->fse == *rf->fsf);

factory->setEnabled(true);
Ice::OutputStream out(communicator);
Expand All @@ -789,7 +777,7 @@ allTests(Test::TestHelper* helper, bool)
factory->setEnabled(false);

rf = dynamic_cast<FObjectReader*>(obj.get())->getF();
test(rf->ae && !rf->af);
test(rf->fse.m == 56 && !rf->fsf);
}
cout << "ok" << endl;

Expand Down Expand Up @@ -1294,23 +1282,23 @@ allTests(Test::TestHelper* helper, bool)

{
FPtr f = make_shared<F>();
f->af = make_shared<A>();
(*f->af)->requiredA = 56;
f->ae = *f->af;
f->fsf = FixedStruct();
(*f->fsf).m = 56;
f->fse = *f->fsf;

Ice::OutputStream out(communicator);
out.startEncapsulation();
out.write(1, make_optional(f));
out.write(2, make_optional(f->ae));
out.write(2, make_optional(f->fse));
out.endEncapsulation();
out.finished(inEncaps);

optional<FixedStruct> ofs;
Ice::InputStream in(communicator, out.getEncoding(), inEncaps);
in.startEncapsulation();
optional<APtr> a;
in.read(2, a);
in.read(2, ofs);
in.endEncapsulation();
test(a && *a && (*a)->requiredA == 56);
test(ofs && (*ofs).m == 56);
}
cout << "ok" << endl;

Expand Down
12 changes: 4 additions & 8 deletions cpp/test/Ice/optional/Test.ice
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,20 +161,20 @@ class OptionalWithCustom

class E
{
A ae;
FixedStruct fse;
}

class F extends E
{
optional(1) A af;
optional(1) FixedStruct fsf;
}

class G1
struct G1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

G1/G2 and G are apparently used for opG below. It's not clear to me what is special with this test, especially now that G1 and G2 are plain structs.

Copy link
Member Author

@InsertCreativityHere InsertCreativityHere Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these G types are for testing the combination of optionals and marshaled-result:

    ["marshaled-result"] G opMG1();                     // These are still optional on my
    ["marshaled-result"] G opMG2(G p1, out G p2);       // branch I haven't gotten to them yet.

We have a 4 marshaled-result tests in total.
The first uses optional structs (which hold a single required byte),
the second uses an optional sequence, the third uses an optional dictionary.

But none of these test marshaled-result with optional members.
Hence, I kept the 4th test, since, that's what it tests. A class which holds a combination of optional and required data members. And this test doesn't seem class specific. All we need is a class which holds some optional things internally.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, maybe we could remove opG().
It seems we added it as part of a bug-fix: Fixed ICE-6602: optionals marshaling bug, so to play it safe I kept it.
I couldn't find any more detailed explanation in the logs, and forgot how to log into JIRA.

{
string a;
}

class G2
struct G2
{
long a;
}
Expand Down Expand Up @@ -278,10 +278,6 @@ interface Initial

void opClassAndUnknownOptional(A p);

void sendOptionalClass(bool req, optional(1) OneOptional o);

void returnOptionalClass(bool req, out optional(1) OneOptional o);

G opG(G g);

void opVoid();
Expand Down
12 changes: 4 additions & 8 deletions cpp/test/Ice/optional/TestAMD.ice
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,20 +161,20 @@ class OptionalWithCustom

class E
{
A ae;
FixedStruct fse;
}

class F extends E
{
optional(1) A af;
optional(1) FixedStruct fsf;
}

class G1
struct G1
{
string a;
}

class G2
struct G2
{
long a;
}
Expand Down Expand Up @@ -278,10 +278,6 @@ interface Initial

void opClassAndUnknownOptional(A p);

void sendOptionalClass(bool req, optional(1) OneOptional o);

void returnOptionalClass(bool req, out optional(1) OneOptional o);

G opG(G g);

void opVoid();
Expand Down
21 changes: 0 additions & 21 deletions cpp/test/Ice/optional/TestAMDI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,27 +408,6 @@ InitialI::opClassAndUnknownOptionalAsync(
response();
}

void
InitialI::sendOptionalClassAsync(
bool,
optional<shared_ptr<Test::OneOptional>>,
function<void()> response,
function<void(exception_ptr)>,
const Ice::Current&)
{
response();
}

void
InitialI::returnOptionalClassAsync(
bool,
function<void(const optional<shared_ptr<Test::OneOptional>>&)> response,
function<void(exception_ptr)>,
const Ice::Current&)
{
response(make_shared<OneOptional>(53));
}

void
InitialI::opGAsync(
shared_ptr<Test::G> g,
Expand Down
13 changes: 0 additions & 13 deletions cpp/test/Ice/optional/TestAMDI.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,19 +261,6 @@ class InitialI final : public Test::Initial
std::function<void(std::exception_ptr)>,
const Ice::Current&) final;

void sendOptionalClassAsync(
bool,
std::optional<std::shared_ptr<Test::OneOptional>>,
std::function<void()>,
std::function<void(std::exception_ptr)>,
const Ice::Current&) final;

void returnOptionalClassAsync(
bool,
std::function<void(const std::optional<std::shared_ptr<Test::OneOptional>>&)>,
std::function<void(std::exception_ptr)>,
const Ice::Current&) final;

void opGAsync(
std::shared_ptr<Test::G>,
std::function<void(const std::shared_ptr<Test::G>&)>,
Expand Down
11 changes: 0 additions & 11 deletions cpp/test/Ice/optional/TestI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,17 +339,6 @@ InitialI::opClassAndUnknownOptional(APtr, const Ice::Current&)
{
}

void
InitialI::sendOptionalClass(bool, optional<OneOptionalPtr>, const Ice::Current&)
{
}

void
InitialI::returnOptionalClass(bool, optional<OneOptionalPtr>& o, const Ice::Current&)
{
o = make_shared<OneOptional>(53);
}

GPtr
InitialI::opG(GPtr g, const Ice::Current&)
{
Expand Down
4 changes: 0 additions & 4 deletions cpp/test/Ice/optional/TestI.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,6 @@ class InitialI : public Test::Initial

virtual void opClassAndUnknownOptional(Test::APtr, const Ice::Current&);

virtual void sendOptionalClass(bool, std::optional<Test::OneOptionalPtr>, const Ice::Current&);

virtual void returnOptionalClass(bool, std::optional<Test::OneOptionalPtr>&, const Ice::Current&);

virtual ::Test::GPtr opG(::Test::GPtr g, const Ice::Current&);

virtual void opVoid(const Ice::Current&);
Expand Down
51 changes: 18 additions & 33 deletions csharp/test/Ice/optional/AllTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,19 +336,6 @@ public class AllTests : global::Test.AllTests
test(cb.obj != null && cb.obj is TestValueReader);
factory.setEnabled(false);

//
// Use the 1.0 encoding with operations whose only class parameters are optional.
//
var oo = new Ice.Optional<Test.OneOptional>(new Test.OneOptional(53));
initial.sendOptionalClass(true, oo);
Test.InitialPrx initial2 = (Test.InitialPrx)initial.ice_encodingVersion(Ice.Util.Encoding_1_0);
initial2.sendOptionalClass(true, oo);

initial.returnOptionalClass(true, out oo);
test(oo.HasValue);
initial2.returnOptionalClass(true, out oo);
test(!oo.HasValue);

Test.G g = new Test.G();
g.gg1Opt = new Ice.Optional<Test.G1>(new Test.G1("gg1Opt"));
g.gg2 = new Test.G2(10);
Expand Down Expand Up @@ -451,16 +438,16 @@ public class AllTests : global::Test.AllTests
}
output.WriteLine("ok");

output.Write("testing marshaling of objects with optional objects...");
output.Write("testing marshaling of objects with optional members...");
output.Flush();
{
Test.F f = new Test.F();

f.af = new Test.A();
f.ae = (Test.A)f.af;
f.fsf = new Test.FixedStruct();
f.fse = (Test.FixedStruct)f.fsf;

Test.F rf = (Test.F)initial.pingPong(f);
test(rf.ae == rf.af.Value);
test(rf.fse == rf.fsf.Value);

factory.setEnabled(true);
os = new Ice.OutputStream(communicator);
Expand All @@ -475,7 +462,7 @@ public class AllTests : global::Test.AllTests
@in.endEncapsulation();
factory.setEnabled(false);
rf = ((FValueReader)rocb.obj).getF();
test(rf.ae != null && !rf.af.HasValue);
test(rf.fse != null && !rf.fsf.HasValue);
}
output.WriteLine("ok");

Expand Down Expand Up @@ -1867,27 +1854,26 @@ public class AllTests : global::Test.AllTests
@in.endEncapsulation();

Test.F f = new Test.F();
f.af = new Test.A();
f.af.Value.requiredA = 56;
f.ae = f.af.Value;
f.fsf = new Test.FixedStruct(56);
f.fse = f.fsf.Value;

os = new OutputStream(communicator);
os.startEncapsulation();
os.writeOptional(1, OptionalFormat.Class);
os.writeValue(f);
os.writeOptional(2, OptionalFormat.Class);
os.writeValue(f.ae);
os.writeOptional(2, OptionalFormat.VSize);
os.writeSize(4);
Test.FixedStruct.ice_write(os, f.fse);
os.endEncapsulation();
inEncaps = os.finished();

@in = new InputStream(communicator, inEncaps);
@in.startEncapsulation();
test(@in.readOptional(2, OptionalFormat.Class));
ReadValueCallbackI rocb = new ReadValueCallbackI();
@in.readValue(rocb.invoke);
test(@in.readOptional(2, OptionalFormat.VSize));
@in.skipSize();
Test.FixedStruct fs1 = Test.FixedStruct.ice_read(@in);
@in.endEncapsulation();
Test.A a = (Test.A)rocb.obj;
test(a != null && a.requiredA == 56);
test(fs1 != null && fs1.m == 56);
}

{
Expand Down Expand Up @@ -1978,6 +1964,7 @@ public class AllTests : global::Test.AllTests
//
// Use the 1.0 encoding with an exception whose only class members are optional.
//
Test.InitialPrx initial2 = (Test.InitialPrx)initial.ice_encodingVersion(Ice.Util.Encoding_1_0);
Ice.Optional<int> a = new Ice.Optional<int>(30);
Ice.Optional<string> b = new Ice.Optional<string>("test");
Ice.Optional<Test.OneOptional> o = new Ice.Optional<Test.OneOptional>(new Test.OneOptional(53));
Expand Down Expand Up @@ -2333,15 +2320,13 @@ public override void read(Ice.InputStream @in)
_f = new Test.F();
@in.startValue();
@in.startSlice();
// Don't read af on purpose
//in.read(1, _f.af);
// Don't read fsf on purpose
//@in.read(1, _f.fsf);
@in.endSlice();
@in.startSlice();
ReadValueCallbackI rocb = new ReadValueCallbackI();
@in.readValue(rocb.invoke);
_f.fse = Test.FixedStruct.ice_read(@in);
@in.endSlice();
@in.endValue();
_f.ae = (Test.A)rocb.obj;
}

public Test.F getF()
Expand Down
Loading