Skip to content

Commit 4577f11

Browse files
emilbon99emilbon99
committed
[ops] - formatting adn testing
1 parent be33854 commit 4577f11

9 files changed

Lines changed: 92 additions & 65 deletions

File tree

‎arc/go/runtime/node/factory.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ import (
2222

2323
// Config provides dependencies and context for creating node instances.
2424
type Config struct {
25-
alamos.Instrumentation
25+
// Module contains the arc module for accessing global state and functions.
26+
Module module.Module
2627
// Node is the IR definition for this node.
2728
Node ir.Node
2829
// State provides access to input/output data and channel I/O.
2930
State *state.Node
30-
// Module contains the arc module for accessing global state and functions.
31-
Module module.Module
31+
alamos.Instrumentation
3232
}
3333

3434
// Factory creates node instances from IR definitions.

‎client/py/synnax/label/__init__.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77
# License, use of this software will be governed by the Apache License, Version 2.0,
88
# included in the file licenses/APL.txt.
99

10-
from synnax.label.types_gen import GoSVCLabel, Key, Label
10+
from synnax.label.types_gen import Key, Label
1111

1212
__all__ = [
13-
"GoSVCLabel",
1413
"Key",
1514
"Label",
1615
]

‎core/pkg/service/arc/runtime/runtime.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ import (
4646

4747
// Config is the configuration for an arc runtime.
4848
type Config struct {
49+
// Module is the compiled arc module that needs to be executed.
50+
//
51+
// [REQUIRED]
52+
Module arc.Module
4953
// Channel is used for retrieving channel information from the cluster.
5054
//
5155
// [REQUIRED]
@@ -59,10 +63,6 @@ type Config struct {
5963
// [REQUIRED]
6064
Status *status.Service
6165
Name string
62-
// Module is the compiled arc module that needs to be executed.
63-
//
64-
// [REQUIRED]
65-
Module arc.Module
6666
}
6767

6868
var (

‎core/pkg/service/framer/calculation/compiler/compile.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ func preProcess(ctx context.Context, cfg Config) (arc.Module, error) {
7070
}
7171

7272
type Module struct {
73+
arc.Module
7374
Channel channel.Channel
7475
StateConfig runtime.ExtendedStateConfig
75-
arc.Module
7676
}
7777

7878
func Compile(ctx context.Context, cfgs ...Config) (Module, error) {

‎oracle/plugin/cpp/json/json.go‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -586,33 +586,6 @@ func (p *Plugin) parseExprForField(field resolution.Field, parent resolution.Typ
586586
return fmt.Sprintf(`parser.field<%s>("%s")`, cppType, jsonName)
587587
}
588588

589-
func (p *Plugin) parseExprForTypeRef(typeRef resolution.TypeRef, cppType, jsonName string, hasDefault bool, data *templateData) string {
590-
if typeRef.TypeParam != nil {
591-
return fmt.Sprintf(`parser.field<%s>("%s")`, typeRef.TypeParam.Name, jsonName)
592-
}
593-
594-
resolved, resolvedOk := typeRef.Resolve(data.table)
595-
if resolvedOk {
596-
if _, isStruct := resolved.Form.(resolution.StructForm); isStruct {
597-
structType := domain.GetName(resolved, "cpp")
598-
if resolved.Namespace != data.rawNs {
599-
targetOutputPath := output.GetPath(resolved, "cpp")
600-
if targetOutputPath != "" {
601-
ns := deriveNamespace(targetOutputPath)
602-
structType = fmt.Sprintf("::%s::%s", ns, structType)
603-
}
604-
}
605-
return fmt.Sprintf(`parser.field<%s>("%s")`, structType, jsonName)
606-
}
607-
}
608-
609-
if mapping := primitiveMapper.Map(typeRef.Name); mapping.TargetType != "" && mapping.TargetType != "void" {
610-
return fmt.Sprintf(`parser.field<%s>("%s")`, cppType, jsonName)
611-
}
612-
613-
return fmt.Sprintf(`parser.field<%s>("%s")`, cppType, jsonName)
614-
}
615-
616589
func (p *Plugin) genericParseExprsForField(field resolution.Field, data *templateData) (jsonParseExpr, structParseExpr string) {
617590
jsonName := toSnakeCase(field.Name)
618591
typeParamName := field.Type.TypeParam.Name

‎x/cpp/pb/pb.h‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,9 @@ namespace x::pb {
2020
/// @param dst The destination container to populate.
2121
/// @param src The source protobuf repeated field.
2222
/// @return x::errors::NIL on success, or the first error encountered.
23-
template <typename CppElem, typename Container, typename PbContainer>
24-
x::errors::Error from_proto_repeated(
25-
Container& dst,
26-
const PbContainer& src
27-
) {
28-
for (const auto& item : src) {
23+
template<typename CppElem, typename Container, typename PbContainer>
24+
x::errors::Error from_proto_repeated(Container &dst, const PbContainer &src) {
25+
for (const auto &item: src) {
2926
auto [v, err] = CppElem::from_proto(item);
3027
if (err) return err;
3128
dst.push_back(v);

‎x/cpp/pb/pb_test.cpp‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,57 +24,55 @@ struct MockProto {
2424
struct MockElement {
2525
int value;
2626

27-
static std::pair<MockElement, errors::Error>
28-
from_proto(const MockProto &pb) {
27+
static std::pair<MockElement, errors::Error> from_proto(const MockProto &pb) {
2928
if (pb.should_fail)
3029
return {{}, errors::Error("test.error", "mock conversion failed")};
3130
return {MockElement{pb.value}, errors::NIL};
3231
}
3332
};
3433

35-
TEST(testpb, testFromProtoRepeatedEmptyContainer) {
34+
TEST(TestPB, testFromProtoRepeatedEmptyContainer) {
3635
std::vector<MockProto> src;
3736
std::vector<MockElement> dst;
38-
auto err = from_proto_repeated<MockElement>(dst, src);
39-
ASSERT_NIL(err);
37+
ASSERT_NIL(from_proto_repeated<MockElement>(dst, src));
4038
ASSERT_EQ(dst.size(), 0);
4139
}
4240

43-
TEST(testpb, testFromProtoRepeatedSingleItem) {
41+
TEST(TestPB, testFromProtoRepeatedSingleItem) {
4442
std::vector<MockProto> src = {{42, false}};
4543
std::vector<MockElement> dst;
46-
auto err = from_proto_repeated<MockElement>(dst, src);
47-
ASSERT_NIL(err);
44+
ASSERT_NIL(from_proto_repeated<MockElement>(dst, src));
4845
ASSERT_EQ(dst.size(), 1);
4946
ASSERT_EQ(dst[0].value, 42);
5047
}
5148

52-
TEST(testpb, testFromProtoRepeatedMultipleItems) {
49+
TEST(TestPB, testFromProtoRepeatedMultipleItems) {
5350
std::vector<MockProto> src = {{1, false}, {2, false}, {3, false}};
5451
std::vector<MockElement> dst;
55-
auto err = from_proto_repeated<MockElement>(dst, src);
56-
ASSERT_NIL(err);
52+
ASSERT_NIL(from_proto_repeated<MockElement>(dst, src));
5753
ASSERT_EQ(dst.size(), 3);
5854
ASSERT_EQ(dst[0].value, 1);
5955
ASSERT_EQ(dst[1].value, 2);
6056
ASSERT_EQ(dst[2].value, 3);
6157
}
6258

63-
TEST(testpb, testFromProtoRepeatedErrorOnFirstItem) {
59+
TEST(TestPB, testFromProtoRepeatedErrorOnFirstItem) {
6460
std::vector<MockProto> src = {{1, true}, {2, false}, {3, false}};
6561
std::vector<MockElement> dst;
66-
auto err = from_proto_repeated<MockElement>(dst, src);
67-
ASSERT_TRUE(err);
68-
ASSERT_MATCHES(err, errors::Error("test.error", ""));
62+
ASSERT_OCCURRED_AS(
63+
from_proto_repeated<MockElement>(dst, src),
64+
errors::Error("test.error", "")
65+
);
6966
ASSERT_EQ(dst.size(), 0);
7067
}
7168

72-
TEST(testpb, testFromProtoRepeatedErrorOnMiddleItem) {
69+
TEST(TestPB, testFromProtoRepeatedErrorOnMiddleItem) {
7370
std::vector<MockProto> src = {{1, false}, {2, true}, {3, false}};
7471
std::vector<MockElement> dst;
75-
auto err = from_proto_repeated<MockElement>(dst, src);
76-
ASSERT_TRUE(err);
77-
ASSERT_MATCHES(err, errors::Error("test.error", ""));
72+
ASSERT_OCCURRED_AS(
73+
from_proto_repeated<MockElement>(dst, src),
74+
errors::Error("test.error", "")
75+
);
7876
ASSERT_EQ(dst.size(), 1);
7977
ASSERT_EQ(dst[0].value, 1);
8078
}

‎x/cpp/test/test.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,13 +464,20 @@ auto assert_occurred_as_p(
464464
}
465465

466466
/// @brief macro asserting that the provided errors::Error is NIL.
467-
#define ASSERT_NIL(expr) ASSERT_FALSE(expr) << expr;
467+
#define ASSERT_NIL(expr) \
468+
do { \
469+
auto _assert_nil_result = (expr); \
470+
ASSERT_FALSE(_assert_nil_result) << _assert_nil_result; \
471+
} while (0)
468472

469473
/// @brief macro asserting that the provided errors::Error is the same as the provided
470474
/// error.
471475
#define ASSERT_OCCURRED_AS(expr, err) \
472-
ASSERT_TRUE(expr) << expr; \
473-
ASSERT_MATCHES(expr, err);
476+
do { \
477+
auto _assert_occurred_as_result = (expr); \
478+
ASSERT_TRUE(_assert_occurred_as_result) << _assert_occurred_as_result; \
479+
ASSERT_MATCHES(_assert_occurred_as_result, err); \
480+
} while (0)
474481

475482
/// @brief macro asserting that the error return as the second item in the pair is the
476483
/// same as the provided error and returning the result value

‎x/cpp/test/test_test.cpp‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// included in the file licenses/APL.txt.
99

1010
#include <atomic>
11+
#include <vector>
1112

1213
#include "gtest/gtest.h"
1314

@@ -169,4 +170,56 @@ TEST_F(XTestTest, TestEventuallyFalseWithCustomTimeout) {
169170
);
170171
t.join();
171172
}
173+
174+
/// @brief ASSERT_NIL should only evaluate its expression once.
175+
/// Regression test for double-evaluation bug.
176+
TEST_F(XTestTest, TestAssertNilSingleEvaluation) {
177+
int eval_count = 0;
178+
auto returns_nil = [&eval_count]() -> errors::Error {
179+
++eval_count;
180+
return errors::NIL;
181+
};
182+
ASSERT_NIL(returns_nil());
183+
EXPECT_EQ(eval_count, 1) << "ASSERT_NIL evaluated expression " << eval_count
184+
<< " times instead of 1";
185+
}
186+
187+
/// @brief ASSERT_OCCURRED_AS should only evaluate its expression once.
188+
/// Regression test for double-evaluation bug.
189+
TEST_F(XTestTest, TestAssertOccurredAsSingleEvaluation) {
190+
int eval_count = 0;
191+
auto returns_error = [&eval_count]() -> errors::Error {
192+
++eval_count;
193+
return errors::Error("test.error", "test error");
194+
};
195+
ASSERT_OCCURRED_AS(returns_error(), errors::Error("test.error", ""));
196+
EXPECT_EQ(eval_count, 1) << "ASSERT_OCCURRED_AS evaluated expression " << eval_count
197+
<< " times instead of 1";
198+
}
199+
200+
/// @brief ASSERT_NIL should only evaluate once with side effects.
201+
/// Verifies that side effects only happen once.
202+
TEST_F(XTestTest, TestAssertNilSideEffects) {
203+
std::vector<int> items;
204+
auto append_and_return_nil = [&items]() -> errors::Error {
205+
items.push_back(42);
206+
return errors::NIL;
207+
};
208+
ASSERT_NIL(append_and_return_nil());
209+
EXPECT_EQ(items.size(), 1)
210+
<< "ASSERT_NIL caused " << items.size() << " side effects instead of 1";
211+
}
212+
213+
/// @brief ASSERT_OCCURRED_AS should only evaluate once with side effects.
214+
/// Verifies that side effects only happen once.
215+
TEST_F(XTestTest, TestAssertOccurredAsSideEffects) {
216+
std::vector<int> items;
217+
auto append_and_return_error = [&items]() -> errors::Error {
218+
items.push_back(42);
219+
return errors::Error("test.error", "test error");
220+
};
221+
ASSERT_OCCURRED_AS(append_and_return_error(), errors::Error("test.error", ""));
222+
EXPECT_EQ(items.size(), 1)
223+
<< "ASSERT_OCCURRED_AS caused " << items.size() << " side effects instead of 1";
224+
}
172225
}

0 commit comments

Comments
 (0)