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

Add the ability to get the current script or project path when using the new test runner #110

Open
alextekartik opened this issue May 13, 2015 · 38 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@alextekartik
Copy link

http://stackoverflow.com/questions/30214563/get-current-script-path-or-current-project-path-using-new-test-runner
I am porting old vm unittest files using the new test package. Some relies on input files in sub directories of my test folder. Before I was using Platform.script to find the location of such files. This works fine when using

$ dart test/my_test.dart

However using

$ pub run test

this is now pointing to a temp folder (tmp/dart_test_xxxx/runInIsolate.dart). I am unable to locate my test input files anymore. I cannot rely on the current path as I might run the test from a different working directory.

Is there a way to find the location of my_test.dart (or event the project root path), from which I could derive the locations of my files?

@nex3 nex3 added type-enhancement A request for a change that isn't a bug status-blocked Blocked from making progress by another (referenced) issue labels May 13, 2015
@nex3
Copy link
Member

nex3 commented May 13, 2015

This is the sort of thing that should be exposed through the API (#48).

@jmesserly
Copy link
Contributor

/sub ... I'm having the same problem (also covered in #123). Our existing tests use Platform.script to load data, making hard to migrate to test runner.

Anecdotally: most packages I've worked on with VM tests end up using data files.

Anyway, I'm guessing I can work around this with dart:mirrors ... instead of Platform.script, find the script's library URI via mirrors.

jmesserly pushed a commit to dart-archive/dev_compiler that referenced this issue Jun 1, 2015
dart_style becomes a dev_dependency, and remove the ability to auto-run it from dart backend, instead use the test formatter
migrate to package:test, which has a few tricky bits:
* expect can only appear inside a test now (dart-lang/test#132)
* mirrors is needed to get the test folder (dart-lang/test#110)
* test `main` can't take arguments. not sure what's up there, but not a big deal for us either.

[email protected]

Review URL: https://codereview.chromium.org/1166683005
@enyo
Copy link

enyo commented Jun 2, 2015

Is there a simple hack/workaround for this, that we can use until it is implemented? (Apart from setting an environment variable)

@alextekartik
Copy link
Author

I use the following ugly workaround (cross posted on SO) that gets me the directory name of the current test script if I'm running it directly or with pub run test. It will definitely break if anything in the implementation changes but I needed this desperately...

library test_utils.test_script_dir;

import 'dart:io';
import 'package:path/path.dart';

// temp workaround using test package
String get testScriptDir {
  String scriptFilePath = Platform.script.toFilePath();
  print(scriptFilePath);
  if (scriptFilePath.endsWith("runInIsolate.dart")) {

    // Let's look for this line:
    // import "file:///path_to_my_test/test_test.dart" as test;

    String importLineBegin = 'import "file://';
    String importLineEnd = '" as test;';
    int importLineBeginLength = importLineBegin.length;

    String scriptContent = new File.fromUri(Platform.script).readAsStringSync();

    int beginIndex = scriptContent.indexOf(importLineBegin);
    if (beginIndex > -1) {
      int endIndex = scriptContent.indexOf(importLineEnd, beginIndex + importLineBeginLength);
      if (endIndex > -1) {
        scriptFilePath = scriptContent.substring(beginIndex + importLineBegin.length, endIndex);
      }
    }
  }
  return dirname(scriptFilePath);
}

@jmesserly
Copy link
Contributor

fyi, I got it working with something a bit simpler:
https://github.com/dart-lang/dev_compiler/blob/master/test/test_util.dart

import 'dart:mirrors';
import 'package:path/path.dart' as path;

final String testDirectory =
    path.dirname((reflectClass(_TestUtils).owner as LibraryMirror).uri.path);

class _TestUtils {}

Basically, get your own library through mirrors and grab the URI path.

@munificent
Copy link
Member

If you don't want to make a fake class, you can also do:

currentMirrorSystem().findLibrary(#your_library).uri.path);

@srawlins
Copy link
Member

Is this a bug/feature of pub run? Should there be a sister issue over at dart-lang/pub?

@nex3
Copy link
Member

nex3 commented Oct 27, 2015

No, if anything, it's an issue with the core libraries. The forthcoming resource/package-spec stuff should solve it, though.

nex3 pushed a commit to dart-lang/sdk that referenced this issue Aug 31, 2016
dart_style becomes a dev_dependency, and remove the ability to auto-run it from dart backend, instead use the test formatter
migrate to package:test, which has a few tricky bits:
* expect can only appear inside a test now (dart-lang/test#132)
* mirrors is needed to get the test folder (dart-lang/test#110)
* test `main` can't take arguments. not sure what's up there, but not a big deal for us either.

[email protected]

Review URL: https://codereview.chromium.org/1166683005
@srawlins
Copy link
Member

@nex3: Regarding your last comment, do you know if it's been solved yet? Or if there is an SDK issue to link to? I'd love an issue to track.

@grouma grouma removed the status-blocked Blocked from making progress by another (referenced) issue label Jun 15, 2018
@polotto
Copy link

polotto commented Jul 26, 2019

I had this problem when I trying read some files in my tests section, to solve that and the problem to open file em multiple platforms, I write the following script:

String retrieveFilePath(String fileName, [String baseDir]){
  var context;
  // get platform context
  if(Platform.isWindows) {
    context = path.Context(style:path.Style.windows);
  } else {
    context = path.Context(style:path.Style.posix);
  }

  // case baseDir not informed, get current script dir
  baseDir ??= path.dirname(Platform.script.path);
  // join dirPath with fileName
  var filePath = context.join(baseDir, fileName);
  // convert Uri to String to make the string treatment more easy
  filePath = context.fromUri(context.normalize(filePath));
  // remove possibles extra paths generated by testing routines
  filePath = path.fromUri(filePath).split('file:').last;

  return filePath;
}

@sigurdm
Copy link
Contributor

sigurdm commented Oct 7, 2021

I think this issue should be taken up again. Seems like the Resource class didn't happen - and dart:mirrors is more or less deprecated.

It would be really nice to have a getter in package:test for the current test script location or some other reliable and intuitive way of getting at the path.

@natebosch
Copy link
Member

natebosch commented Oct 7, 2021

@sigurdm directory like test/data/ or something similar?

One concern with exposing a path only is that it isn't useful on the web. If we make APIs for reading files, we could potentially make it work on all platforms and build systems. Differences in the directory layout is an ongoing pain point between dart test and bazel test and a specific file reading API could work around that.

@sigurdm
Copy link
Contributor

sigurdm commented Oct 8, 2021

Yeah - that would work for me!

@Jonas-Sander
Copy link

test/data/ is exactly what I want to use this for aswell

@natebosch
Copy link
Member

Capturing some thoughts about this after some conversations with @jakemac53 and @nshahan

An alternative could be a builder (eventually a macro) that inlines file content to a const String or list of bytes in Dart source. Even if we had macros today, this alternative might not be satisfying since it doesn't allow for reading a large file, handling it, and then freeing it from memory before reading the next.

I had proposed a hardcoded directory test/data/ for this, but paths could also be relative to:

  • The package root
  • The currently running test suite.
  • The file that makes the call.

I think we prefer either a hardcoded directory or the package root. Relative to the file that makes this too complicated to implement, and relative to the currently running test suite could be confusing.

Use the package root:
Pros

  • Feels intuitive to Dart users, not magical. Matches CWD for external test runner usage, so easy transition to make.
  • Maximally flexible, can read pubspec.yaml which is a common use case to check during tests.
  • Can lay out data and tests in the same directory instead of spreading them across trees.
  • The "package root" concept is more fully baked and consistent than it used to be. It used to be only a convention, but now it's also encoded in the package config.

Cons

  • Need to make decisions about how to limit what is read. Do we allow reading .dart_tool/ or do we need to try to block some things?
  • May need more implementation effort to normalize across environment. If we don't limit what files can be read, then either we need to take care that bazel exposes the same files (possibly through a multi directory overlay if outputs are spread out) or accept that there are still differences in behavior.
  • Cannot automatically include the conventional directory as data in bazel, or as a required output with build_runner test.

Use test/data/:
Pros

  • No need to limit what is read, anything in this directory can is assumed intended to be readable.
  • Easy to implement consistently in all build systems.
  • Can automatically include as data in bazel, and as an output in build_runner test.
  • Convention makes it easier to jump between packages and know where to look.

Cons

  • Some existing test data would need to move.
  • Can't colocate test files and data files.
  • Can't read pubspec.yaml.
  • More magic (although a name like readTestData('foo.txt') is easy to associate with test/data/foo.txt)

@natebosch
Copy link
Member

An example of the type of code written to work around the environment differences:

https://dart-review.googlesource.com/c/sdk/+/242162

Neither of the proposals above would solve this issue. Either could solve the issue if there was a step outside of running the tests which copied or symlinked the files into the package somewhere. I could imagine doing that in bazel in place of setting up the environment variables, but we wouldn't have a good solution for this case externally.

@natebosch
Copy link
Member

natebosch commented Jun 8, 2022

There is some potential for aligning this with our internal bazel build rules and having a close coupling with the data argument to the test target.

b/235365551

@jakemac53
Copy link
Contributor

jakemac53 commented Jun 8, 2022

Yes, I think we should read from $RUNFILES/<bazel-package-root> which would contain anything given in data. These aren't compile time deps but runtime deps.

@jakemac53
Copy link
Contributor

Another thing to consider here is reading data from other packages, likely via package: uris. This should also work in bazel but you will have to set up appropriate dependencies on the targets that expose those files, I believe that could just be file groups which you depend on in your data dependencies as well.

@jamesderlin
Copy link
Contributor

jamesderlin commented Feb 26, 2023

Another hacky alternative is to extract the path from the stack trace. I think this works (for non-web cases)?

import 'package:stack_trace/stack_trace.dart' as stacktrace;

String currentDartFilePath({bool packageRelative = false}) {
  var caller = stacktrace.Frame.caller(1);
  return packageRelative ? caller.library : caller.uri.toFilePath();
}

@natebosch
Copy link
Member

It's very plausible that things like parsing the stack trace will work in some environments and not others. I don't know the specific behavior in bazel internally.

@dcharkes
Copy link
Contributor

I also ran into this, and also made a workaround.

Without knowing about this discussion, I also gravitated towards using the package root.

That being said, a test/data/ approach would have probably also worked.

FWIW, my code (only tested on VM, not in browser, and not tested with Bazel).

/// Test files are run in a variety of ways, find this package root in all.
///
/// Test files can be run from source from any working directory. The Dart SDK
/// `tools/test.py` runs them from the root of the SDK for example.
///
/// Test files can be run from dill from the root of package. `package:test`
/// does this.
Uri findPackageRoot(String packageName) {
  final script = Platform.script;
  final fileName = script.name;
  if (fileName.endsWith('_test.dart')) {
    // We're likely running from source.
    var directory = script.resolve('.');
    while (true) {
      final dirName = directory.name;
      if (dirName == packageName) {
        return directory;
      }
      final parent = directory.resolve('..');
      if (parent == directory) break;
      directory = parent;
    }
  } else if (fileName.endsWith('.dill')) {
    final cwd = Directory.current.uri;
    final dirName = cwd.name;
    if (dirName == packageName) {
      return cwd;
    }
  }
  throw StateError("Could not find package root for package '$packageName'. "
      'Tried finding the package root via Platform.script '
      "'${Platform.script.toFilePath()}' and Directory.current "
      "'${Directory.current.uri.toFilePath()}'.");
}

Uri packageUri = findPackageRoot('c_compiler');

extension on Uri {
  String get name => pathSegments.where((e) => e != '').last;
}

@jakemac53
Copy link
Contributor

Fwiw, to get the root of the current package it might be a bit more robust to use package:package_config and Isolate.packageConfigUri, and then you can just look up the root of your package by name (assuming you know the name of the package, this can't be easily fully generalized). This will also only work on the VM though.

@dcharkes
Copy link
Contributor

That being said, a test/data/ approach would have probably also worked.

My test data contains dart projects that contain tests, so nesting it inside test/ doesn't work for my use case. Instead, I'm now using test_data/. (If we had a way to skip test files inside test/data/ when doing dart test but not skip them when doing cd test/data/foo_project/ && dart test that would also work for my use case.)

@jakemac53
Copy link
Contributor

We do have the filename configuration option https://github.com/dart-lang/test/blob/master/pkgs/test/doc/configuration.md#filename. You should be able to use that in the top level package to include only the dirs you want, and then assuming that test/data/foo_project is itself a package, dart test under there should still run all the tests.

@dcharkes
Copy link
Contributor

We do have the filename configuration option https://github.com/dart-lang/test/blob/master/pkgs/test/doc/configuration.md#filename. You should be able to use that in the top level package to include only the dirs you want, and then assuming that test/data/foo_project is itself a package, dart test under there should still run all the tests.

Don't you mean paths? The documentation for filename says it only takes into account the base-name, not the full path (and separators).

The paths are not globs, so we have to enumerate everything under test/ that is not test/data/. That works: dart-lang/native#69.

@jakemac53
Copy link
Contributor

The documentation for filename says it only takes into account the base-name, not the full path (and separators).

Ah I didn't catch that it was basename only. Using paths should also work although what you really want is like an exclude_paths.

@matanlurey
Copy link
Contributor

Can this be part of the programmatic API, that is, a pkg/test(_api|core):

/// Returns the package root (the directory with `pubspec.yaml`) for the currently running test.
///
/// This is a partial replacement for the functionality of `Platform.script.path`, but works regardless of whether
/// the script is run directly (`dart run test/my_test.dart`) or through the test runner (`dart test test/my_test.dart`).
String get currentPackageRoot {
  // ...
}

I would find this super helpful, and it would make adopting something like dart test --workspace possible.

@jakemac53
Copy link
Contributor

jakemac53 commented Sep 10, 2024

What would the expected behavior be for non-cli tests? For example web tests or flutter tests.

Or even for cli tests, it gets weird in bazel or build_runner test

@natebosch
Copy link
Member

Can this be part of the programmatic API, that is, a pkg/test(_api|core):

We'd prefer it not be, since this isn't a path we can provide in google3.

What would you do with that information after you have it? And would you be skipping those tests in google3?

@matanlurey
Copy link
Contributor

What would the expected behavior be for non-cli tests? For example web tests or flutter tests.

Flutter tests do have access to the file system (it's a thin embedder on top of the Dart VM), but ignoring that - I think it would be OK to either give the root anyway (assuming it's possible) - it won't be useful information but it could still be provided, or to have the API throw UnsupportedError.

We'd prefer it not be, since this isn't a path we can provide in google3.
Or even for cli tests, it gets weird in bazel or build_runner test

I'm worried we're making perfect enemy of the good here - the workarounds needed to get functionality that needs to 95-99% of the time just run on the Dart VM independent of google3 are significant, and fixable. If pkg/test has an "embedder" API, it could allow providing a resolver for this method, and Bazel/g3 could use it, otherwise it could just throw Unsupported.

@natebosch
Copy link
Member

natebosch commented Sep 10, 2024

otherwise it could just throw Unsupported.

Historically we had a fair amount of pain during rolls into google3 when there are tests that rely on the particulars of external pub project structure and fail in google3. Do you expect usage of this particular API to remain limited to tests which are easy and safe to exclude from google3 testing?

Flutter tests do have access to the file system (it's a thin embedder on top of the Dart VM), but ignoring that - I think it would be OK to either give the root anyway (assuming it's possible) - it won't be useful information but it could still be provided, or to have the API throw UnsupportedError.

Would it be sufficient to provide the package name for the currently running test and require the test embedder to do something meaningful from the name? From #110 (comment) the main concern in your use case is knowing what package to look up right?

That would leave the decision to introduce non-cross-compatible behavior outside of the test package.

@matanlurey
Copy link
Contributor

You could even not export this in the google3 build

@matanlurey
Copy link
Contributor

Historically we had a fair amount of pain during rolls into google3 when there are tests that rely on the particulars of external pub project structure and fail in google3. Do you expect usage of this particular API to remain limited to tests which are easy and safe to exclude from google3 testing?

You could even not export this in the google3 build, i.e. its not part of the g3 API.

Would it be sufficient to provide the package name for the currently running test and require the test embedder to do something meaningful from the name? From #110 (comment) the main concern in your use case is knowing what package to look up right?

That would leave the decision to introduce non-cross-compatible behavior outside of the test package.

I think the answer is "yes", assuming the default embedder does it :)

@natebosch
Copy link
Member

natebosch commented Sep 10, 2024

#2245 is precedent for exposing information about the package config location to test embedders (through a somewhat suspect communication channel which works around limitations in providing this information through the VM service while the isolate is paused).

Edit: The package config URI that we know about here likely isn't useful for flutter test, so probably we'd still want to make only the package name available.

@natebosch
Copy link
Member

Hmm, we might not even reliably know the package name.

@jakemac53
Copy link
Contributor

One idea here is to just expose the relative path to the current test script. So, something like test/foo_test.dart. We can do that in an agnostic way, already have the information in the right places, and just need to plumb it through.

For most tests, you can then do Uri.base.resolve(testPath) to get the path to the current test. For web tests, we might need to set the base URI using the html tag, which I think is what Uri.base would give you, so this would enable loading assets on the web too potentially (but need to confirm).

@jakemac53
Copy link
Contributor

jakemac53 commented Sep 11, 2024

One snag with the relative path solution is supporting running test files directly, although we could try to infer it from Platform.script. We could just have the API throw an UnsupportedError though which would be pretty reasonable, or return null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests