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 allow host access to Graaljs under flags #2022

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

tokou
Copy link
Contributor

@tokou tokou commented Sep 5, 2024

Context

#1996 (comment)

Proposed changes

  • I also added MAESTRO_CLI_DANGEROUS_GRAALJS_ALLOW_ALL_ACCESS for allowAllAccess()
  • I added a disabled integration test for the case where the env var needs to be set as I don't think it's possible to mock System.env.
  • (Unrelated) Fix to .idea/.gitignore

Testing

  • Unit tests for truthness of env values
  • Integration tests

Issues fixed

Linked #1949
Linked #1996
Fixes #1388

@bartekpacia
Copy link
Contributor

Tests failing because of false positive #2005

Copy link
Contributor

@bartekpacia bartekpacia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, let's just resolve the few nites I made above.

@amanjeetsingh150
Copy link
Collaborator

Thanks for this PR @tokou 🙌 planning to merge this soon after the 1.39.0 release for 1.40.0. Adding a label for it.

@amanjeetsingh150 amanjeetsingh150 added the v1.40.0 Release 1.40.0 label Sep 19, 2024
@amanjeetsingh150
Copy link
Collaborator

Just to confirm @tokou does this now enable importing different js modules? Mentioned in following issue:

#1637

@tokou
Copy link
Contributor Author

tokou commented Sep 19, 2024

Oh I haven't noticed that issue @amanjeetsingh150
I'll give it a look and see if this fixes it and if not will add another env var for it.

@tiagodread
Copy link

Oh I haven't noticed that issue @amanjeetsingh150 I'll give it a look and see if this fixes it and if not will add another env var for it.

Hey @tokou I've built your branch locally and executed the scenario described here

./maestro-cli/build/install/maestro/bin/maestro test ../mobile_app/e2e_test/flows/sample.yaml

appId: com.sample
jsEngine: graaljs
env:
  MAESTRO_CLI_DANGEROUS_GRAALJS_ALLOW_HOST_ACCESS: true
  MAESTRO_CLI_DANGEROUS_GRAALJS_ALLOW_CLASS_LOOKUP: true
---
- runScript: 
    file: main.js

- evalScript: ${console.log(output.url)}
// main.js
import { env } from "./env.mjs";

const testImport = () => {
  console.log(`${env.URL}`);
  return env.URL;
};

output.url = testImport();
// env.mjs
export const env = {
  URL: "https://example.com",
  ANOTHER_URL: "https://another-example.com",
};

Result:

SyntaxError: main.js:2:0 Expected an operand but found import                   
import { env } from "./env.mjs";                                                
^                                                                               
main.js:2:21 Expected ; but found ./env.mjs                                     
import { env } from "./env.mjs";                                                
                     ^                                                          
                                                                                
        at org.graalvm.polyglot.Context.eval(Context.java:399)                  
        at maestro.js.GraalJsEngine.evaluateScript(GraalJsEngine.kt:72)         
        at maestro.js.GraalJsEngine.evaluateScript(GraalJsEngine.kt:24)
        at maestro.orchestra.Orchestra.runScriptCommand(Orchestra.kt:421)
        at maestro.orchestra.Orchestra.executeCommand(Orchestra.kt:289)
        at maestro.orchestra.Orchestra.executeCommands(Orchestra.kt:194)
        at maestro.orchestra.Orchestra.runFlow(Orchestra.kt:128)
        at maestro.cli.runner.MaestroCommandRunner.runCommands(MaestroCommandRunner.kt:177)
        at maestro.cli.runner.TestRunner$runSingle$result$1.invoke(TestRunner.kt:62)
        at maestro.cli.runner.TestRunner$runSingle$result$1.invoke(TestRunner.kt:53)
        at maestro.cli.runner.TestRunner.runCatching(TestRunner.kt:160)
        at maestro.cli.runner.TestRunner.runSingle(TestRunner.kt:53)
        at maestro.cli.command.TestCommand.runSingleFlow(TestCommand.kt:352)
        at maestro.cli.command.TestCommand.access$runSingleFlow(TestCommand.kt:63)
        at maestro.cli.command.TestCommand$runShardSuite$2.invoke(TestCommand.kt:326)
        at maestro.cli.command.TestCommand$runShardSuite$2.invoke(TestCommand.kt:297)
        at maestro.cli.session.MaestroSessionManager.newSession(MaestroSessionManager.kt:102)
        at maestro.cli.session.MaestroSessionManager.newSession$default(MaestroSessionManager.kt:52)
        at maestro.cli.command.TestCommand.runShardSuite(TestCommand.kt:297)
        at maestro.cli.command.TestCommand.access$runShardSuite(TestCommand.kt:63)
        at maestro.cli.command.TestCommand$handleSessions$1$results$1$1.invokeSuspend(TestCommand.kt:265)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
        at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:111)
        at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:99)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:585)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:802)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:706)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:693)


==== Debug output (logs & screenshots) ====

Seems that isn't fixing #1637 ?!

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

Successfully merging this pull request may close these issues.

Update GraalJS engine to latest version
4 participants