Investigate inconsistencies between running `’dot shorthands: single-stepping’` test with `ModuleFormat.ddc` v.s. with `ModuleFormat.amd`

[derekxu16]

dwds/test/instances/common/dot_shorthands_common.dart currently contains the following lines:

if (provider.ddcModuleFormat == ModuleFormat.ddc)
  '$scriptBasename:$lineB:20', // on '2'

An investigation should be performed to figure out why the if guard is needed.

[bkonyi]

@srujzs is this something you could take a look into at some point? If not, could you direct this to who you think would have the most context here? :)

[derekxu16]

I'm not sure if it's the cause of this issue, but DWDS tests don't have something like the VM Service tests' pauseOnStart, so I think there can be races depending on when onBreakpoint sets its breakpoint.

[srujzs]

This specific discrepancy is for a field:

webdev/fixtures/_experiment/web/main.dart

Line 127 in d2e5593

c = .two; // lineE

The field two is lazily initialized, but by the time we start stepping, it has already been initialized. This is because main runs testDotShorthands periodically:

webdev/fixtures/_experiment/web/main.dart

Line 30 in d2e5593

testDotShorthands();

After it's been initialized, we never step into the body that initialized it. The getter and the method naturally don't have that issue. With the DDC library bundle format, presumably due to some invariants around hot reload, we still use a getter to fetch the cached value:

    /*main.C.two*/static get two() {
      if (this[__v_two] === void 0) {
        this[__v_two] = new main.C.new(2);
      }
      this[__v_two];
      Object.defineProperty(this, "two", {
        get() {
          return this[__v_two]; // here
        }
      });
      return this.two;
    }

This matters because that means in the body of get() we have an opportunity to stop at a breakpoint, and hence the divergent behavior.

We can avoid this by just using evaluate to run the execution instead of a periodic timer. I'll need to refactor the test a bit, but I believe that should fix it.

[srujzs]

Summarizing thoughts from the sync:

The stepping tests are somewhat underspecified, because it isn't clear what each step should produce. For example, we stop at ';' which seems odd. The AMD module format might be more "correct" as fields should indeed only produce a breakpoint into the initialization once and not on following gets, but there is no concrete specification here. For now, #2687 tries to unify the tests, but these are essentially regression tests.

In the future, we should try to better specify steps and breakable points (maybe even be consistent with the VM!) and revamp the source maps we emit for DDC.