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.

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

Enable "File > open" tests for Replay tests #144

Closed
jamesderlin opened this issue Jun 18, 2020 · 4 comments
Closed

Enable "File > open" tests for Replay tests #144

jamesderlin opened this issue Jun 18, 2020 · 4 comments

Comments

@jamesderlin
Copy link
Collaborator

I added a MemoryFileSystem-based implementation of RandomAccessFile in #136. I enabled File > Open tests for test/memory_test.dart, but I did not notice that various other tests depend on MemoryFileSystem and also explicitly disable File > open tests.

Attempting to enable File > open tests in test/replay_test.dart generates lots of errors that I don't yet understand:

test/replay_test.dart: Replay common File open READ RandomAccessFile read rerun read [E]
  type 'List<dynamic>' is not a subtype of type 'Uint8List' of 'input'
  package:file/src/backends/record_replay/codecs.dart                          Passthrough.convert
  package:file/src/backends/record_replay/replay_proxy_mixin.dart 133:30       ReplayProxyMixin.noSuchMethod
  package:file/src/backends/record_replay/replay_random_access_file.dart 17:7  ReplayRandomAccessFile.readSync
  test/common_tests.dart 1825:41                                               runCommonTests.<fn>.<fn>.<fn>.testRandomAccessFileOperations.<fn>.<fn>.<fn>

test/replay_test.dart: Replay common File open READ RandomAccessFile read rerun readIntoWithBufferLargerThanContent [E]
  No matching invocation found: readIntoSync(Instance(length:1024) of '_GrowableList'0, null, )
  package:file/src/backends/record_replay/replay_proxy_mixin.dart 125:7        ReplayProxyMixin.noSuchMethod
  package:file/src/backends/record_replay/replay_random_access_file.dart 17:7  ReplayRandomAccessFile.readIntoSync
  test/common_tests.dart 1832:37                                               runCommonTests.<fn>.<fn>.<fn>.testRandomAccessFileOperations.<fn>.<fn>.<fn>
@jamesderlin
Copy link
Collaborator Author

I looked into the failures.

Failure type 1:

type 'List' is not a subtype of type 'Uint8List' of 'input'

ReplayRandomAccessFile currently uses const Passthrough<Uint8List>() for the #readSync converter. The input is a List<int> that isn't a Uint8List. I don't yet understand why.

Failure type 2:

No matching invocation found: readIntoSync(Instance(length:1024) of '_GrowableList', 0, null)

This happens because on the recording side, we write the event to the manifest after invoking the method, but the method mutates the supplied List argument, so we write the mutated argument, not the original one. The replay side then fails to find the corresponding entry since the List argument no longer matches.

@jamesderlin
Copy link
Collaborator Author

ReplayRandomAccessFile currently uses const Passthrough<Uint8List>() for the #readSync converter. The input is a List<int> that isn't a Uint8List. I don't yet understand why.

The data is serialized as a JSON array of integers, so I think an explicit conversion to Uint8List should be expected.

This happens because on the recording side, we write the event to the manifest after invoking the method,

Maybe RecordingProxyMixin.noSuchMethod could eagerly encode the argument lists to JSON and pass them directly to LiveMethodEvent.

@jamesderlin
Copy link
Collaborator Author

Ugh, there is yet another problem in that there's currently no way for record/replay to replicate mutations to method arguments. The recording site probably would need to encode the arguments before invocation and after, and then the replay side would need to figure out how to update arguments in-place. Or maybe there could be some less general mechanism since all of this would be needed only for RandomFileAccess.readInto/readIntoSync.

dnfield pushed a commit that referenced this issue Jun 23, 2020
* Enable more "File > open" tests

When I added a `MemoryFileSystem`-based implementation of
`RandomAccessFile` in #136,
I enabled `File > open` tests for `test/memory_test.dart` but did not
notice that various other tests depend on `MemoryFileSystem` and also
explicitly disabled them.

Enable them for `chroot_test.dart` and for `recording_test.dart`.
Enabling them for `replay_test.dart` currently does not work due to
other failures that I don't yet understand (see
#144).

* Fix type error when attempting to enable "File > open" replay tests

Fix `ReplayFile.read`/`readSync` to explicitly convert the input
(read from a JSON integer array) into a `Uint8List`.

Fixes one of the failures in #144.

* Fix analysis error

* Disable "File > open" replay tests more selectively

Record/replay currently doesn't work with `RandomAccessFile.readInto`,
but we can enable other `RandomAccessFile` tests.
@jamesderlin
Copy link
Collaborator Author

This is no longer relevant since record/replay was removed by #162.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant