Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@ztone
Copy link
Contributor

@ztone ztone commented Jun 5, 2014

This PR adds the same engine script tests for Mono that are testing the Roslyn Engine.

This fixes #663 (at least partly)

Two things purposely left out to keep this PR cleaner and are part of other issues.

  • Mono Engine does not throw CompileException and needs to be implemented. Currently the ConsoleReportWriter is displaying but "swallowing" the exceptions. This issue is more connected to Mono error reporting #664.
  • Mono Engine does not support IsCompleteSubmission and therefore has no tests around that feature. The engine tests around this should be included in the PR that fixes Mono REPL multiline #665.

@adamralph
Copy link
Contributor

@ztone thanks for this!

Just throwing this out there as an idea: how about we use the same code file for both RoslynScriptEngineTests and MonoScriptEngineTests?

We could name it ScriptEngineTests and link it into each project. For now we'd have to use #if to omit certain tests for mono, which is quite ugly, but at least it would allow us, at a glance, to see the difference in the unit tests for each of the two types.

There is of course a fundamental difference between the two which is the use of the RoslynScriptEngine/MonoScriptEngine and RoslynTestScriptEngine/MonoTestScriptEngine types but this could be done using one #if at the top

#if MONO
using ScriptEngine = RoslynScriptEngine;
using TestScriptEngine = RoslynTestScriptEngine;
#else
using ScriptEngine = MonoScriptEngine;
using TestScriptEngine = MonoTestScriptEngine;
#endif

@scriptcs/core thoughts?

@ztone
Copy link
Contributor Author

ztone commented Jun 5, 2014

@adamralph I somewhat expected this to be raised as the tests just have minor differences and are in strict sense dups. Having a shared tests project will insure that both engines support the same logic and keeps them in sync.

However, engines have minor differences, e.g. comments after evaluation is not as well supported in Mono as in Roslyn, logic around when IsCompleteSubmission is not the same for engines and probably some other stuff.

There are pros and cons with both approaches. I'm good either way ... just wanted the engine tested.

@filipw
Copy link
Member

filipw commented Jun 11, 2014

Great work. I agree with @adamralph 100% but I think priority 1 for now is to get any tests in - right now there is hardly any safety blanket around the Mono engine.
Let's think of reconciliation between the engine tests later on, as of now I'm going to pull this in. Thanks a lot @ztone

filipw added a commit that referenced this pull request Jun 11, 2014
@filipw filipw merged commit 033b617 into scriptcs:dev Jun 11, 2014
@adamralph
Copy link
Contributor

👍

@adamralph adamralph mentioned this pull request Jun 11, 2014
@ztone ztone deleted the mono-eval-tests branch June 11, 2014 18:15
@adamralph adamralph modified the milestone: v0.10 Jun 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mono REPL multiline Add tests for Mono engine

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.