Refactor isolate test #16933
Refactor isolate test #16933
Conversation
dnfield
added
the
waiting for tree to go green
|
Looks like the mac test is going to timeout. /cc @yjbanov - the felt test on mac for this run has already taken > 51 minutes and appears to not be making progress. I'm going to ignore the test for this change since the change cannot possibly be impacting web code, it's a C++ only test based change. @yjbanov @nturgut - maybe we could have the web tests skip running if the only changes are to .cc|.h|.mm|.m|*.java files? |
| FML_DISALLOW_COPY_AND_ASSIGN(AutoIsolateShutdown); | ||
| }; | ||
|
|
||
| static void RunDartCodeInIsolate(DartVMRef& vm_ref, |
There was a problem hiding this comment.
As soon as this header gets included in more than one translation unit, there are going to be duplicate symbol errors. The implementation needs to be in its own TU.
| result = std::move(root_isolate); | ||
| } | ||
|
|
||
| static std::unique_ptr<AutoIsolateShutdown> RunDartCodeInIsolate( |
There was a problem hiding this comment.
Ditto about needing to be in its own TU.
| public: | ||
| AutoIsolateShutdown() = default; | ||
|
|
||
| AutoIsolateShutdown(std::shared_ptr<DartIsolate> isolate, |
There was a problem hiding this comment.
Now that this is a general purpose utility, please OOL these like in the rest of the engine codebase.
There was a problem hiding this comment.
Fixed these in #16941
|
Sorry about the delay in the review. Can we please fix the OOL-ing of the static methods in a follow up patch? |
|
I'll follow up on this. |
Split from #16824
This is intended to make it easier for other test harnesses to launch a Dart VM. See linked PR for example of using it.