Merge pull request #328 from smalruby/309-stop-self-when

feat: stop self.when

Author: takaokouji

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

@@ -0,0 +1,70 @@
+# Unit and Integration test for interconverting between "Code" and "Ruby"
+
+- User Story: smalruby/smalruby3-gui#309
+- Status: accepted
+- Deciders: takaokouji
+- Date: 2023-01-08T14:50:56+0900
+
+Sorry, this is written in Japanese.
+
+# ブロックとRubyの相互変換の自動テストの実装方針
+
+現在、ブロックとRubyの相互変換の自動テストは、次の2つのファイルに記載している。
+
+- `test/unit/lib/ruby-to-blocks-converter/*.test.js`
+- `test/integration/ruby-tab/*.test.js`
+
+前者はいわゆる単体テストで、すべてがNode.js上で実行されて軽くて速いが、Rubyからブロックへの変換処理のみが対象なので、一部しかテストできない。
+
+後者はいわゆるシステムテストで、テストコードはNode.jsだが、処理の実行はヘッダレスChrome上で行うため、重くて遅いが、ブロックとRubyとの相互変換処理が対象なので、全体をテストできる。
+
+どちらの自動テストもメンテナンスし続けられればベストだが、作業時間が限られるため、現実的な方法で自動テストをメンテナンスしていきたい。
+
+## 判断基準や制限
+
+* 十分にテストできること
+  * デグレを検知できること
+* テストの実装が簡単であること
+* 実行時間が短いこと
+
+## 選択肢
+
+* (現行) 単体テストとシステムテストの両方を実装する
+* 単体テストのみ実装する
+* システムテストのみ実装する
+* 単体テストはエラーケースのみ、システムテストはそれ以外
+
+## 決定
+
+**採用:** 単体テストはエラーケースのみ、システムテストはそれ以外
+
+* 「システムテストのみ実装する」と迷った
+* 有限時間でできるのはこれしかなかった
+* コーナーケースのチェックが甘くなってしまうため、手動テストで確認すべきところは文章などで残しておかないといけない
+  * 単体テスト側のソースコードにコメントとして書いていくことにする (時間があれば単体テストとして実装する余地を残すため)
+
+## 各選択肢の良い点と悪い点
+
+### 単体テストとシステムテストの両方を実装する
+
+* `+` 単体テストでRubyからブロックへの変換処理のコーナーケースを高速にテストでき、システムテストで俯瞰的なテストができ、十分にテストできる
+* `-` ブロックの内部構造を把握する必要があり単体テストの実装が難しく、時間がかかる
+
+### 単体テストのみ実装する
+
+* `+` 単体テストでRubyからブロックへの変換処理のコーナーケースを高速にテストできる
+* `+` システムテストを実装する必要がなく、作業時間が短縮できる
+* `-` ブロックの内部構造を把握する必要があり単体テストの実装が難しく、時間がかかる
+* `-` ブロックからRubyへの変換処理のテストを実装できず、デグレが発生する可能性が高いし、毎回、手作業でRubyへの変換処理のテストをする必要がある
+
+### システムテストのみ実装する
+
+* `+` 単体テストを実装する手間がかからない
+* `+` システムテストの実装そのものは簡単
+* `-` コーナーケースを実装する場合にテストの実行時間が長くなってしまう。結果として、コーナーケースは最低限の実装になったり、実装しなかったりする
+
+### 単体テストはエラーケースのみ、システムテストはそれ以外
+
+* `+` エラーケースのみのRubyからブロックへの変換処理の単体テストは実装が簡単
+* `+` システムテストでは、最も煩雑なエラーケースを扱わないことで、すべてのケースを実装する場合に比べて、実行時間が短くできる。想定では50%くらい違う。
+* `-` Rubyからブロックへの変換処理の正常系のうち、コーナーケースのチェックが甘くなってしまい、細かい処理でデグレが発生する可能性がある

src/lib/ruby-generator/control.js

@@ -51,7 +51,7 @@ export default function (Generator) {
 
     Generator.control_start_as_clone = function (block) {
         block.isStatement = true;
-        return `${Generator.spriteName()}.when(:start_as_a_clone) do\n`;
+        return `when_start_as_a_clone do\n`;
     };
 
     Generator.control_create_clone_of = function (block) {

src/lib/ruby-generator/event.js

@@ -12,31 +12,31 @@ export default function (Generator) {
     Generator.event_whenkeypressed = function (block) {
         block.isStatement = true;
         const key = Generator.quote_(Generator.getFieldValue(block, 'KEY_OPTION') || null);
-        return `${Generator.spriteName()}.when(:key_pressed, ${key}) do\n`;
+        return `when_key_pressed(${key}) do\n`;
     };
 
     Generator.event_whenthisspriteclicked = function (block) {
         block.isStatement = true;
-        return `${Generator.spriteName()}.when(:clicked) do\n`;
+        return `when_clicked do\n`;
     };
 
     Generator.event_whenbackdropswitchesto = function (block) {
         block.isStatement = true;
         const backdrop = Generator.quote_(Generator.getFieldValue(block, 'BACKDROP') || '');
-        return `${Generator.spriteName()}.when(:backdrop_switches, ${backdrop}) do\n`;
+        return `when_backdrop_switches(${backdrop}) do\n`;
     };
 
     Generator.event_whengreaterthan = function (block) {
         block.isStatement = true;
         const lh = Generator.quote_(Generator.getFieldValue(block, 'WHENGREATERTHANMENU').toLowerCase() || '');
         const rh = Generator.valueToCode(block, 'VALUE', Generator.ORDER_NONE) || '0';
-        return `${Generator.spriteName()}.when(:greater_than, ${lh}, ${rh}) do\n`;
+        return `when_greater_than(${lh}, ${rh}) do\n`;
     };
 
     Generator.event_whenbroadcastreceived = function (block) {
         block.isStatement = true;
         const message = Generator.getFieldValue(block, 'BROADCAST_OPTION');
-        return `${Generator.spriteName()}.when(:receive, ${Generator.quote_(message)}) do\n`;
+        return `when_receive(${Generator.quote_(message)}) do\n`;
     };
 
     Generator.event_broadcast = function (block) {
@@ -56,7 +56,7 @@ export default function (Generator) {
 
     Generator.event_whenstageclicked = function (block) {
         block.isStatement = true;
-        return `Stage.when(:clicked) do\n`;
+        return `when_clicked do\n`;
     };
 
     return Generator;

src/lib/ruby-generator/microbit.js

@@ -7,7 +7,7 @@ export default function (Generator) {
     Generator.microbit_whenButtonPressed = function (block) {
         block.isStatement = true;
         const btn = Generator.valueToCode(block, 'BTN', Generator.ORDER_NONE) || null;
-        return `${Generator.spriteName()}.when(:microbit_button_pressed, ${btn}) do\n`;
+        return `microbit.when_button_pressed(${btn}) do\n`;
     };
 
     Generator.microbit_isButtonPressed = function (block) {
@@ -18,7 +18,7 @@ export default function (Generator) {
     Generator.microbit_whenGesture = function (block) {
         block.isStatement = true;
         const gesture = Generator.valueToCode(block, 'GESTURE', Generator.ORDER_NONE) || null;
-        return `${Generator.spriteName()}.when(:microbit_gesture, ${gesture}) do\n`;
+        return `microbit.when(${gesture}) do\n`;
     };
 
     Generator.microbit_displaySymbol = function (block) {
@@ -39,7 +39,7 @@ export default function (Generator) {
     Generator.microbit_whenTilted = function (block) {
         block.isStatement = true;
         const direction = Generator.valueToCode(block, 'DIRECTION', Generator.ORDER_NONE) || null;
-        return `${Generator.spriteName()}.when(:microbit_tilted, ${direction}) do\n`;
+        return `microbit.when_tilted(${direction}) do\n`;
     };
 
     Generator.microbit_isTilted = function (block) {
@@ -55,7 +55,7 @@ export default function (Generator) {
     Generator.microbit_whenPinConnected = function (block) {
         block.isStatement = true;
         const pin = Generator.valueToCode(block, 'PIN', Generator.ORDER_NONE) || null;
-        return `${Generator.spriteName()}.when(:microbit_pin_connected, ${pin}) do\n`;
+        return `microbit.when_pin_connected(${pin}) do\n`;
     };
 
     Generator.microbit_menu_buttons = function (block) {

src/lib/ruby-generator/video.js

@@ -6,13 +6,13 @@
 export default function (Generator) {
     Generator.videoSensing_whenMotionGreaterThan = function (block) {
         block.isStatement = true;
-        const rh = Generator.valueToCode(block, 'REFERENCE', Generator.ORDER_NONE) || 0;
-        return `${Generator.spriteName()}.when(:video_motion_greater_than, ${rh}) do\n`;
+        const reference = Generator.valueToCode(block, 'REFERENCE', Generator.ORDER_NONE) || 0;
+        return `video_sensing.when_video_motion_greater_than(${reference}) do\n`;
     };
 
     Generator.videoSensing_videoToggle = function (block) {
         const videoState = Generator.valueToCode(block, 'VIDEO_STATE', Generator.ORDER_NONE);
-        return `video_turn(${videoState})\n`;
+        return `video_sensing.video_turn(${videoState})\n`;
     };
 
     Generator.videoSensing_menu_VIDEO_STATE = function (block) {
@@ -22,28 +22,23 @@ export default function (Generator) {
 
     Generator.videoSensing_setVideoTransparency = function (block) {
         const transparency = Generator.valueToCode(block, 'TRANSPARENCY', Generator.ORDER_NONE) || 0;
-        return `self.video_transparency = ${transparency}\n`;
+        return `video_sensing.video_transparency = ${transparency}\n`;
     };
 
     Generator.videoSensing_videoOn = function (block) {
         const attribute = Generator.valueToCode(block, 'ATTRIBUTE', Generator.ORDER_NONE);
         const subject = Generator.valueToCode(block, 'SUBJECT', Generator.ORDER_NONE);
-        return [`${subject}video_${attribute}`, Generator.ORDER_ATOMIC];
+        return [`video_sensing.video_on(${attribute}, ${subject})`, Generator.ORDER_ATOMIC];
     };
 
     Generator.videoSensing_menu_ATTRIBUTE = function (block) {
-        const attribute = Generator.getFieldValue(block, 'ATTRIBUTE') || 'motion';
+        const attribute = Generator.quote_(Generator.getFieldValue(block, 'ATTRIBUTE') || 'motion');
         return [attribute, Generator.ORDER_ATOMIC];
     };
 
     Generator.videoSensing_menu_SUBJECT = function (block) {
-        const subject = Generator.getFieldValue(block, 'SUBJECT') || 'this sprite';
-        if (subject === 'Stage') {
-            return ['stage.', Generator.ORDER_ATOMIC];
-        } else if (subject === 'this sprite') {
-            return ['', Generator.ORDER_ATOMIC];
-        }
-        return [`${subject}.`, Generator.ORDER_ATOMIC];
+        const subject = Generator.quote_(Generator.getFieldValue(block, 'SUBJECT') || 'this sprite');
+        return [subject, Generator.ORDER_ATOMIC];
     };
 
     return Generator;

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

@@ -68,17 +68,6 @@ const ControlConverter = {
                     block = this._createBlock('control_delete_this_clone', 'statement');
                 }
                 break;
-            case 'when':
-                if (args.length === 1 &&
-                    args[0].type === 'sym' && args[0].value === 'start_as_a_clone' &&
-                    rubyBlockArgs && rubyBlockArgs.length === 0) {
-                    block = this._createBlock('control_start_as_clone', 'hat');
-                    if (this._isBlock(rubyBlock)) {
-                        rubyBlock.parent = block.id;
-                        block.next = rubyBlock.id;
-                    }
-                }
-                break;
             }
         } else if (this._isNumberOrBlock(receiver)) {
             switch (name) {
@@ -122,6 +111,33 @@ const ControlConverter = {
         }
         this._addSubstack(block, statement);
         return block;
+    },
+
+    register: function (converter) {
+        converter.registerCallMethodWithBlock('self', 'when_start_as_a_clone', 0, 0, params => {
+            const {rubyBlock} = params;
+
+            const block = converter.createBlock('control_start_as_clone', 'hat');
+            converter.setParent(rubyBlock, block);
+            return block;
+        });
+
+        // backward compatibility
+        converter.registerCallMethodWithBlock('self', 'when', 1, 0, params => {
+            const {args} = params;
+
+            if (args[0].type !== 'sym') return null;
+
+            switch (args[0].value) {
+            case 'start_as_a_clone':
+                return converter.callMethod(
+                    params.receiver, 'when_start_as_a_clone', params.args.slice(1),
+                    params.rubyBlockArgs, params.rubyBlock, params.node
+                );
+            }
+
+            return null;
+        });
     }
 };