Skip to content

Commit 2b8056c

Browse files
takaokoujitakaokouji
authored
Merge pull request #328 from smalruby/309-stop-self-when
feat: stop self.when
2 parents 76e3782 + 6b6711a commit 2b8056c

File tree

16 files changed

+1278
-1864
lines changed

16 files changed

+1278
-1864
lines changed

‎docs/adr/0003-test-for-interconverting-block-and-ruby.md

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# Unit and Integration test for interconverting between "Code" and "Ruby"
2+
3+
- User Story: smalruby/smalruby3-gui#309
4+
- Status: accepted
5+
- Deciders: takaokouji
6+
- Date: 2023-01-08T14:50:56+0900
7+
8+
Sorry, this is written in Japanese.
9+
10+
# ブロックとRubyの相互変換の自動テストの実装方針
11+
12+
現在、ブロックとRubyの相互変換の自動テストは、次の2つのファイルに記載している。
13+
14+
- `test/unit/lib/ruby-to-blocks-converter/*.test.js`
15+
- `test/integration/ruby-tab/*.test.js`
16+
17+
前者はいわゆる単体テストで、すべてがNode.js上で実行されて軽くて速いが、Rubyからブロックへの変換処理のみが対象なので、一部しかテストできない。
18+
19+
後者はいわゆるシステムテストで、テストコードはNode.jsだが、処理の実行はヘッダレスChrome上で行うため、重くて遅いが、ブロックとRubyとの相互変換処理が対象なので、全体をテストできる。
20+
21+
どちらの自動テストもメンテナンスし続けられればベストだが、作業時間が限られるため、現実的な方法で自動テストをメンテナンスしていきたい。
22+
23+
## 判断基準や制限
24+
25+
* 十分にテストできること
26+
* デグレを検知できること
27+
* テストの実装が簡単であること
28+
* 実行時間が短いこと
29+
30+
## 選択肢
31+
32+
* (現行) 単体テストとシステムテストの両方を実装する
33+
* 単体テストのみ実装する
34+
* システムテストのみ実装する
35+
* 単体テストはエラーケースのみ、システムテストはそれ以外
36+
37+
## 決定
38+
39+
**採用:** 単体テストはエラーケースのみ、システムテストはそれ以外
40+
41+
* 「システムテストのみ実装する」と迷った
42+
* 有限時間でできるのはこれしかなかった
43+
* コーナーケースのチェックが甘くなってしまうため、手動テストで確認すべきところは文章などで残しておかないといけない
44+
* 単体テスト側のソースコードにコメントとして書いていくことにする (時間があれば単体テストとして実装する余地を残すため)
45+
46+
## 各選択肢の良い点と悪い点
47+
48+
### 単体テストとシステムテストの両方を実装する
49+
50+
* `+` 単体テストでRubyからブロックへの変換処理のコーナーケースを高速にテストでき、システムテストで俯瞰的なテストができ、十分にテストできる
51+
* `-` ブロックの内部構造を把握する必要があり単体テストの実装が難しく、時間がかかる
52+
53+
### 単体テストのみ実装する
54+
55+
* `+` 単体テストでRubyからブロックへの変換処理のコーナーケースを高速にテストできる
56+
* `+` システムテストを実装する必要がなく、作業時間が短縮できる
57+
* `-` ブロックの内部構造を把握する必要があり単体テストの実装が難しく、時間がかかる
58+
* `-` ブロックからRubyへの変換処理のテストを実装できず、デグレが発生する可能性が高いし、毎回、手作業でRubyへの変換処理のテストをする必要がある
59+
60+
### システムテストのみ実装する
61+
62+
* `+` 単体テストを実装する手間がかからない
63+
* `+` システムテストの実装そのものは簡単
64+
* `-` コーナーケースを実装する場合にテストの実行時間が長くなってしまう。結果として、コーナーケースは最低限の実装になったり、実装しなかったりする
65+
66+
### 単体テストはエラーケースのみ、システムテストはそれ以外
67+
68+
* `+` エラーケースのみのRubyからブロックへの変換処理の単体テストは実装が簡単
69+
* `+` システムテストでは、最も煩雑なエラーケースを扱わないことで、すべてのケースを実装する場合に比べて、実行時間が短くできる。想定では50%くらい違う。
70+
* `-` Rubyからブロックへの変換処理の正常系のうち、コーナーケースのチェックが甘くなってしまい、細かい処理でデグレが発生する可能性がある

‎src/lib/ruby-generator/control.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export default function (Generator) {
5151

5252
Generator.control_start_as_clone = function (block) {
5353
block.isStatement = true;
54-
return `${Generator.spriteName()}.when(:start_as_a_clone) do\n`;
54+
return `when_start_as_a_clone do\n`;
5555
};
5656

5757
Generator.control_create_clone_of = function (block) {

‎src/lib/ruby-generator/event.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,31 +12,31 @@ export default function (Generator) {
1212
Generator.event_whenkeypressed = function (block) {
1313
block.isStatement = true;
1414
const key = Generator.quote_(Generator.getFieldValue(block, 'KEY_OPTION') || null);
15-
return `${Generator.spriteName()}.when(:key_pressed, ${key}) do\n`;
15+
return `when_key_pressed(${key}) do\n`;
1616
};
1717

1818
Generator.event_whenthisspriteclicked = function (block) {
1919
block.isStatement = true;
20-
return `${Generator.spriteName()}.when(:clicked) do\n`;
20+
return `when_clicked do\n`;
2121
};
2222

2323
Generator.event_whenbackdropswitchesto = function (block) {
2424
block.isStatement = true;
2525
const backdrop = Generator.quote_(Generator.getFieldValue(block, 'BACKDROP') || '');
26-
return `${Generator.spriteName()}.when(:backdrop_switches, ${backdrop}) do\n`;
26+
return `when_backdrop_switches(${backdrop}) do\n`;
2727
};
2828

2929
Generator.event_whengreaterthan = function (block) {
3030
block.isStatement = true;
3131
const lh = Generator.quote_(Generator.getFieldValue(block, 'WHENGREATERTHANMENU').toLowerCase() || '');
3232
const rh = Generator.valueToCode(block, 'VALUE', Generator.ORDER_NONE) || '0';
33-
return `${Generator.spriteName()}.when(:greater_than, ${lh}, ${rh}) do\n`;
33+
return `when_greater_than(${lh}, ${rh}) do\n`;
3434
};
3535

3636
Generator.event_whenbroadcastreceived = function (block) {
3737
block.isStatement = true;
3838
const message = Generator.getFieldValue(block, 'BROADCAST_OPTION');
39-
return `${Generator.spriteName()}.when(:receive, ${Generator.quote_(message)}) do\n`;
39+
return `when_receive(${Generator.quote_(message)}) do\n`;
4040
};
4141

4242
Generator.event_broadcast = function (block) {
@@ -56,7 +56,7 @@ export default function (Generator) {
5656

5757
Generator.event_whenstageclicked = function (block) {
5858
block.isStatement = true;
59-
return `Stage.when(:clicked) do\n`;
59+
return `when_clicked do\n`;
6060
};
6161

6262
return Generator;

‎src/lib/ruby-generator/microbit.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export default function (Generator) {
77
Generator.microbit_whenButtonPressed = function (block) {
88
block.isStatement = true;
99
const btn = Generator.valueToCode(block, 'BTN', Generator.ORDER_NONE) || null;
10-
return `${Generator.spriteName()}.when(:microbit_button_pressed, ${btn}) do\n`;
10+
return `microbit.when_button_pressed(${btn}) do\n`;
1111
};
1212

1313
Generator.microbit_isButtonPressed = function (block) {
@@ -18,7 +18,7 @@ export default function (Generator) {
1818
Generator.microbit_whenGesture = function (block) {
1919
block.isStatement = true;
2020
const gesture = Generator.valueToCode(block, 'GESTURE', Generator.ORDER_NONE) || null;
21-
return `${Generator.spriteName()}.when(:microbit_gesture, ${gesture}) do\n`;
21+
return `microbit.when(${gesture}) do\n`;
2222
};
2323

2424
Generator.microbit_displaySymbol = function (block) {
@@ -39,7 +39,7 @@ export default function (Generator) {
3939
Generator.microbit_whenTilted = function (block) {
4040
block.isStatement = true;
4141
const direction = Generator.valueToCode(block, 'DIRECTION', Generator.ORDER_NONE) || null;
42-
return `${Generator.spriteName()}.when(:microbit_tilted, ${direction}) do\n`;
42+
return `microbit.when_tilted(${direction}) do\n`;
4343
};
4444

4545
Generator.microbit_isTilted = function (block) {
@@ -55,7 +55,7 @@ export default function (Generator) {
5555
Generator.microbit_whenPinConnected = function (block) {
5656
block.isStatement = true;
5757
const pin = Generator.valueToCode(block, 'PIN', Generator.ORDER_NONE) || null;
58-
return `${Generator.spriteName()}.when(:microbit_pin_connected, ${pin}) do\n`;
58+
return `microbit.when_pin_connected(${pin}) do\n`;
5959
};
6060

6161
Generator.microbit_menu_buttons = function (block) {

‎src/lib/ruby-generator/video.js

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
export default function (Generator) {
77
Generator.videoSensing_whenMotionGreaterThan = function (block) {
88
block.isStatement = true;
9-
const rh = Generator.valueToCode(block, 'REFERENCE', Generator.ORDER_NONE) || 0;
10-
return `${Generator.spriteName()}.when(:video_motion_greater_than, ${rh}) do\n`;
9+
const reference = Generator.valueToCode(block, 'REFERENCE', Generator.ORDER_NONE) || 0;
10+
return `video_sensing.when_video_motion_greater_than(${reference}) do\n`;
1111
};
1212

1313
Generator.videoSensing_videoToggle = function (block) {
1414
const videoState = Generator.valueToCode(block, 'VIDEO_STATE', Generator.ORDER_NONE);
15-
return `video_turn(${videoState})\n`;
15+
return `video_sensing.video_turn(${videoState})\n`;
1616
};
1717

1818
Generator.videoSensing_menu_VIDEO_STATE = function (block) {
@@ -22,28 +22,23 @@ export default function (Generator) {
2222

2323
Generator.videoSensing_setVideoTransparency = function (block) {
2424
const transparency = Generator.valueToCode(block, 'TRANSPARENCY', Generator.ORDER_NONE) || 0;
25-
return `self.video_transparency = ${transparency}\n`;
25+
return `video_sensing.video_transparency = ${transparency}\n`;
2626
};
2727

2828
Generator.videoSensing_videoOn = function (block) {
2929
const attribute = Generator.valueToCode(block, 'ATTRIBUTE', Generator.ORDER_NONE);
3030
const subject = Generator.valueToCode(block, 'SUBJECT', Generator.ORDER_NONE);
31-
return [`${subject}video_${attribute}`, Generator.ORDER_ATOMIC];
31+
return [`video_sensing.video_on(${attribute}, ${subject})`, Generator.ORDER_ATOMIC];
3232
};
3333

3434
Generator.videoSensing_menu_ATTRIBUTE = function (block) {
35-
const attribute = Generator.getFieldValue(block, 'ATTRIBUTE') || 'motion';
35+
const attribute = Generator.quote_(Generator.getFieldValue(block, 'ATTRIBUTE') || 'motion');
3636
return [attribute, Generator.ORDER_ATOMIC];
3737
};
3838

3939
Generator.videoSensing_menu_SUBJECT = function (block) {
40-
const subject = Generator.getFieldValue(block, 'SUBJECT') || 'this sprite';
41-
if (subject === 'Stage') {
42-
return ['stage.', Generator.ORDER_ATOMIC];
43-
} else if (subject === 'this sprite') {
44-
return ['', Generator.ORDER_ATOMIC];
45-
}
46-
return [`${subject}.`, Generator.ORDER_ATOMIC];
40+
const subject = Generator.quote_(Generator.getFieldValue(block, 'SUBJECT') || 'this sprite');
41+
return [subject, Generator.ORDER_ATOMIC];
4742
};
4843

4944
return Generator;

‎src/lib/ruby-to-blocks-converter/control.js

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,6 @@ const ControlConverter = {
6868
block = this._createBlock('control_delete_this_clone', 'statement');
6969
}
7070
break;
71-
case 'when':
72-
if (args.length === 1 &&
73-
args[0].type === 'sym' && args[0].value === 'start_as_a_clone' &&
74-
rubyBlockArgs && rubyBlockArgs.length === 0) {
75-
block = this._createBlock('control_start_as_clone', 'hat');
76-
if (this._isBlock(rubyBlock)) {
77-
rubyBlock.parent = block.id;
78-
block.next = rubyBlock.id;
79-
}
80-
}
81-
break;
8271
}
8372
} else if (this._isNumberOrBlock(receiver)) {
8473
switch (name) {
@@ -122,6 +111,33 @@ const ControlConverter = {
122111
}
123112
this._addSubstack(block, statement);
124113
return block;
114+
},
115+
116+
register: function (converter) {
117+
converter.registerCallMethodWithBlock('self', 'when_start_as_a_clone', 0, 0, params => {
118+
const {rubyBlock} = params;
119+
120+
const block = converter.createBlock('control_start_as_clone', 'hat');
121+
converter.setParent(rubyBlock, block);
122+
return block;
123+
});
124+
125+
// backward compatibility
126+
converter.registerCallMethodWithBlock('self', 'when', 1, 0, params => {
127+
const {args} = params;
128+
129+
if (args[0].type !== 'sym') return null;
130+
131+
switch (args[0].value) {
132+
case 'start_as_a_clone':
133+
return converter.callMethod(
134+
params.receiver, 'when_start_as_a_clone', params.args.slice(1),
135+
params.rubyBlockArgs, params.rubyBlock, params.node
136+
);
137+
}
138+
139+
return null;
140+
});
125141
}
126142
};
127143

0 commit comments

Comments
 (0)