removed docgen/test/single_library_test: Skip # Uses js_helper

removed docgen/test/single_library_test: Skip # Uses js_helper

BUG=
R=efortuna@google.com

Committed: https://code.google.com/p/dart/source/detail?r=25348

[janicejl, 2013-07-22 19:23:56]

I pushed the change previously, and tests were failing on buildbot. So I
reverted that change. 

The error given on build bot is: 

FAILED: none-vm release_ia32 pkg/docgen/test/single_library_test
Expected: pass 
Actual: fail
CommandOutput[vm]:

stdout:
unittest-suite-wait-for-done
Error: Cannot read "xcodebuild/lib/_internal/lib/js_helper.dart" (OS Error: No
such file or directory, errno = 2).

which was the same output as before, which I think is why I originally put that
on the pkg.status file. 

However, I do not get this when I run tools/test.py. 

I would appreciate any insight about this. 

Thanks, 
Janice

[janicejl, 2013-07-22 19:29:18]

On 2013/07/22 19:23:56, janicejl wrote:
> I pushed the change previously, and tests were failing on buildbot. So I
> reverted that change. 
> 
> The error given on build bot is: 
> 
> FAILED: none-vm release_ia32 pkg/docgen/test/single_library_test
> Expected: pass 
> Actual: fail
> CommandOutput[vm]:
> 
> stdout:
> unittest-suite-wait-for-done
> Error: Cannot read "xcodebuild/lib/_internal/lib/js_helper.dart" (OS Error: No
> such file or directory, errno = 2).
> 
> which was the same output as before, which I think is why I originally put
that
> on the pkg.status file. 
> 
> However, I do not get this when I run tools/test.py. 
> 
> I would appreciate any insight about this. 
> 
> Thanks, 
> Janice

I think the main reason is that buildbot is running the test without the
--use-sdk option. 

Buildbot: python tools/test.py -mrelease -t60
pkg/docgen/test/single_library_test

Is there a way I can tell build bot to use the option --use-sdk? 

Thanks,
Janice

[Emily Fortuna, 2013-07-22 19:39:34]

Janice, if you use the reproduction command in the log file on the buildbot, you
can get it to reproduce on your local machine, no problem. I just rebuilt and
got the test to fail on the first try using the command I copied from the log
file: "python tools/test.py -mrelease -t60 pkg/docgen/test/single_library_test".

So, you're right. It doesn't use --use-sdk since that's not an argument that was
passed to it when it was run.

Why do you need the --use-sdk argument vs the current setup?  Specifying
--use-sdk can only be done for an entire buildbot test run, not really on a
per-test basis.

[ahe, 2013-07-22 20:09:13]

On 2013/07/22 19:39:34, Emily Fortuna wrote:
> Janice, if you use the reproduction command in the log file on the buildbot,
you
> can get it to reproduce on your local machine, no problem. I just rebuilt and
> got the test to fail on the first try using the command I copied from the log
> file: "python tools/test.py -mrelease -t60
pkg/docgen/test/single_library_test".
> 
> So, you're right. It doesn't use --use-sdk since that's not an argument that
was
> passed to it when it was run.
> 
> Why do you need the --use-sdk argument vs the current setup?  Specifying
> --use-sdk can only be done for an entire buildbot test run, not really on a
> per-test basis.

Before you enable this test, please to make sure to address this TODO:

      // TODO(janicejl): Instead of creating a new file, should use an in-memory
      // file for creating the mirrorSystem. Example of the util function in 
      // sdk/lib/_internal/compiler/implementation/mirrors/mirrors_util.dart. 
      // Example of the test is in file mirrors_lookup_test.dart

It is *not* acceptable to write files directly to the current directory on the
build bot, even for a short period of time. Also, you may accidentally overwrite
your co-workers test files because you use such a generic name as "test.dart".
That's not polite ;-)

Here is a fix for setting the SDK root:

diff --git a/dart/pkg/docgen/lib/docgen.dart b/dart/pkg/docgen/lib/docgen.dart
index 415102a..026ea71 100644
--- a/dart/pkg/docgen/lib/docgen.dart
+++ b/dart/pkg/docgen/lib/docgen.dart
@@ -160,7 +160,10 @@ Future<MirrorSystem> getMirrorSystem(List<String> args,
String packageRoot,
   } else {
     // If DART_SDK is not defined in the environment,
     // assuming the dart executable is from the Dart SDK folder inside bin.
-    sdkRoot = path.dirname(path.dirname(new Options().executable));
+    sdkRoot = currentDirectory
+        // Do not use path.toUri.  It is broken.
+        .resolve(nativeToUriPath(new Options().script))
+        .resolve('../../../sdk');
     logger.info('SDK Root: ${sdkRoot}');
   }
 
I'd strongly recommend against using the path package at all.  As far as I can
tell, it is not a good replacement for Uris, and it's toUri method is broken.

Microsoft explains rather well why "shared library code should not use the
GetCurrentDirectory", see:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa364934(v=vs.85).aspx

[janicejl, 2013-07-22 21:52:43]

I have modified docgen to use the sdk in dart-repo rather than in the out folder
as the sdk root when generating the mirror system. 

Also modified the test to create a file in a temporary directory, so that it
does not run the risk of overwriting another test.

[Emily Fortuna, 2013-07-22 23:09:10]

https://codereview.chromium.org/19888005/diff/9001/pkg/docgen/test/single_lib...
File pkg/docgen/test/single_library_test.dart (right):

https://codereview.chromium.org/19888005/diff/9001/pkg/docgen/test/single_lib...
pkg/docgen/test/single_library_test.dart:15: Directory.current = temporaryDir;
how about instead of setting your current directory to the temporary directory,
just use the absolute path to refer to it? 

var file = new File(path.join(temporaryDir.path, 'temp.dart'));

[janicejl, 2013-07-22 23:27:45]

https://codereview.chromium.org/19888005/diff/9001/pkg/docgen/test/single_lib...
File pkg/docgen/test/single_library_test.dart (right):

https://codereview.chromium.org/19888005/diff/9001/pkg/docgen/test/single_lib...
pkg/docgen/test/single_library_test.dart:15: Directory.current = temporaryDir;
On 2013/07/22 23:09:10, Emily Fortuna wrote:
> how about instead of setting your current directory to the temporary
directory,
> just use the absolute path to refer to it? 
> 
> var file = new File(path.join(temporaryDir.path, 'temp.dart'));

Done.

[Emily Fortuna, 2013-07-22 23:42:35]

https://codereview.chromium.org/19888005/diff/14001/pkg/docgen/test/single_li...
File pkg/docgen/test/single_library_test.dart (right):

https://codereview.chromium.org/19888005/diff/14001/pkg/docgen/test/single_li...
pkg/docgen/test/single_library_test.dart:14: var file = new
File(path.join(temporaryDir.path, 'temp.dart'));
you might create a variable called fileName rather than appending the same path 
3 times.

[janicejl, 2013-07-23 16:56:06]

https://codereview.chromium.org/19888005/diff/14001/pkg/docgen/test/single_li...
File pkg/docgen/test/single_library_test.dart (right):

https://codereview.chromium.org/19888005/diff/14001/pkg/docgen/test/single_li...
pkg/docgen/test/single_library_test.dart:14: var file = new
File(path.join(temporaryDir.path, 'temp.dart'));
On 2013/07/22 23:42:35, Emily Fortuna wrote:
> you might create a variable called fileName rather than appending the same
path 
> 3 times.

Done.

[Emily Fortuna, 2013-07-23 16:57:24]

lgtm

[janicejl, 2013-07-23 17:31:26]

Committed patchset #5 manually as r25348 (presubmit successful).