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

isolateIds parameter does not work #533

Open
gogolon opened this issue Aug 21, 2024 · 3 comments
Open

isolateIds parameter does not work #533

gogolon opened this issue Aug 21, 2024 · 3 comments

Comments

@gogolon
Copy link
Contributor

gogolon commented Aug 21, 2024

Doc comment for the collect function states:
If [isolateIds] is set, the coverage gathering will be restricted to only those VM isolates.
This does not seem to be true. I'm trying to collect coverage from a single isolate, but the filter simply does not work and code executed on all isolates is included in the report.

Reproduction

  1. Create a new dart project with dart create isolate_coverage_sample
  2. Replace the code inside bin/isolate_coverage_sample.dart with the code below:
import 'dart:developer';
import 'dart:isolate';

import 'package:coverage/coverage.dart';
import 'package:isolate_coverage_sample/calculate.dart';
import 'package:isolate_coverage_sample/isolate_calculate.dart';

Future<void> main(List<String> arguments) async {
  calculate(); // This function is included by default in packages created using dart create

  final isolate = await Isolate.spawn(
    (_) => isolateCalculate(),
    null,
    paused: true,
  );
  final isolateId = Service.getIsolateId(isolate)!;
  final info = await Service.controlWebServer(enable: true);
  isolate.resume(isolate.pauseCapability!);

  final coverage = await collect(
    info.serverUri!,
    true,
    false,
    false,
    {'isolate_coverage_sample'},
    isolateIds: {isolateId},
  );

  print(coverage.entries);
}
  1. Add a file lib/isolate_calculate.dart containing this code:
int isolateCalculate() {
  return 6 * 7;
}
  1. While at the package's root, run:
    dart --pause-isolates-on-exit bin/isolate_coverage_sample.dart
  2. Inspect the entries on the output. The file package:isolate_coverage_sample/calculate.dart is included in the coverage even though it was called on the main isolate.
@liamappelbe
Copy link
Contributor

This is due to isolate groups. isolateIds (and its documentation) predates the introduction of isolate groups, so it's pretty outdated. The coverage counters are shared between all the isolates in a group (for performance reasons), so what this is actually doing is filtering the coverage to all the isolates in the same group as the given isolate.

In fact, unless you're using Isolate.spawnUri(), that isolate group probably includes your entire app, which is great for performance, but means the isolateIds filter is essentially useless these days.

I can't think of a simple/efficient way of restoring the old behavior either, so the only actionable thing here is to either update the documentation or delete that arg.

@gogolon Do you have a use case that requires the old behavior?

@mosuem mosuem transferred this issue from dart-archive/coverage Aug 28, 2024
@gogolon
Copy link
Contributor Author

gogolon commented Aug 29, 2024

I thought that I have a use case, but maybe I was wrong. If coverage is collected after an isolate exits (but there are other isolates running), does the report contain coverage from the isolate that exited earlier? I see that _getAllCoverage iterates over all isolates and collects coverage for each of them. Therefore I assumed that if an isolate has already exited, its coverage will not be included in the report. I ran a quick test and the opposite seems to be true - the coverage from the exited isolate seems to be included. Is it related to isolate groups? Is it enough for just one isolate per group to be alive at the moment of collection in order to include coverage from all isolates that ever existed within the group?

Due to my (false?) assumption, I was trying to pause the exiting isolates and collect coverage from them separately, using the isolateIds parameter.

@liamappelbe
Copy link
Contributor

Is it enough for just one isolate per group to be alive at the moment of collection in order to include coverage from all isolates that ever existed within the group?

Yeah, that makes sense to me. The coverage counters are shared between all isolates in the group, so even if one of the isolates has exited, the counters it was incrementing are still there.

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

No branches or pull requests

3 participants