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

CI: Enable sanity test for native Runner #8724

Merged
merged 8 commits into from
Jan 12, 2024
Merged

Conversation

mwu-tow
Copy link
Contributor

@mwu-tow mwu-tow commented Jan 10, 2024

Pull Request Description

If the native image of the runner was built, it will receive some basic sanity test. Then, an espresso-enabled variant of the runner will be compiled and tested as well.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@mwu-tow mwu-tow added the CI: No changelog needed Do not require a changelog entry for this PR. label Jan 10, 2024
@mwu-tow mwu-tow self-assigned this Jan 10, 2024
.arg("engine-runner/buildNativeImage")
.run_ok()
.await?;
runner_sanity_test(&self.repo_root, Some(enso_java)).await?;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we also run sanity check without espresso?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is run first:

            runner_sanity_test(&self.repo_root, None).await?;
            ide_ci::fs::remove_file_if_exists(&self.repo_root.runner)?;
            let enso_java = "espresso";
            sbt.command()?
                .env(ENSO_JAVA, enso_java)
                .arg("engine-runner/buildNativeImage")
                .run_ok()
                .await?;
            runner_sanity_test(&self.repo_root, Some(enso_java)).await?;

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand. The snippet you provide runs with "espresso". Why are we not running also the following:

runner_sanity_test(&self.repo_root, None).await?;

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Akirathan

I don't understand. The snippet you provide runs with "espresso". Why are we not running also the following:

We are, this is the first line from my snippet.

Copy link
Member

Choose a reason for hiding this comment

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

I guess the runner image without espresso has already been assembled before we get to this snippet. Then:

  • the snippet runs runner_sanity_test without espresso
  • removes runner
  • builds sbt engine-runner/buildNativeImage again with espresso
  • runs the runner_sanity_test again

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Jan 11, 2024

The runner with espresso has been successfully built here.

Why there are two lines

"/runner/_work/enso/enso/built-distribution/enso-engine-2024.1.1-dev-linux-amd64/enso-2024.1.1-dev" ENSO_JAVA="espresso" "/runner/_work/enso/enso/runner" "--run" "/runner/_work/enso/enso/engine/runner/src/test/resources/Factorial.enso" "6"

next to each other:

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Jan 11, 2024

The runner without Espresso seems to be built at line 1212. Where is it tested?

@Akirathan
Copy link
Member

The runner without Espresso seems to be built at line 1212. Where is it tested?

That is part of "Building project-manager distribution and Native Image" done in:

debug!("Bulding project-manager distribution and Native Image");
. This was never tested. This native image build was there for a long time, just to ensure that the native image build succeeds.

@Akirathan
Copy link
Member

Akirathan commented Jan 11, 2024

The runner with espresso has been successfully built here.

Why there are two lines

"/runner/_work/enso/enso/built-distribution/enso-engine-2024.1.1-dev-linux-amd64/enso-2024.1.1-dev" ENSO_JAVA="espresso" "/runner/_work/enso/enso/runner" "--run" "/runner/_work/enso/enso/engine/runner/src/test/resources/Factorial.enso" "6"

next to each other:

The first line informs about starting the process (ide_ci::program::command: Spawning. at https://github.com/enso-org/enso/actions/runs/7474612603/job/20341133392?pr=8724#step:10:16931). And the second one tells that the process has exited with return code 0 (ide_ci::program::command: close at https://github.com/enso-org/enso/actions/runs/7474612603/job/20341133392?pr=8724#step:10:16936)

GitHub
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.
GitHub
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

@Akirathan
Copy link
Member

Akirathan commented Jan 11, 2024

There are three native image builds now.

The first one is on the line 1135, build from command sbt -Dbench.compileOnly=true "all buildEngineDistribution engine-runner/assembly engine-runner/buildNativeImage verifyLicensePackages" (the command is started on the line 863.

The second one is on 2032, build from the command sbt test, that is started on the line 1310. sbt test builds native image, because the launcher/test depends on buildNativeImage (See

enso/build.sbt

Line 2136 in 31a0dce

.dependsOn(buildNativeImage)
).

The third one is on 16824, build from the command ENSO_JAVA=espresso sbt engine-runner/buildNativeImage that is started on the line 16773.

Altogether, we build 3 native images: 2 images of engine runner - one with ENSO_JAVA=espresso and the other one without it, and the third is a native image of launcher.

@Akirathan
Copy link
Member

Akirathan commented Jan 11, 2024

The runner without Espresso seems to be built at line 1212. Where is it tested?

The runner without Enso is tested on the line 16762, right after dry-run benchmarks finished.

@mwu-tow
Copy link
Contributor Author

mwu-tow commented Jan 11, 2024

@Akirathan @JaroslavTulach
I believe all the questions were already answered between you two? If there's anything I've missed, please ping me.

@mwu-tow mwu-tow marked this pull request as ready for review January 11, 2024 11:01
@mwu-tow mwu-tow merged commit 4776a83 into develop Jan 12, 2024
27 of 28 checks passed
@mwu-tow mwu-tow deleted the wip/mwu/native-runner-test branch January 12, 2024 03:59
@mwu-tow mwu-tow mentioned this pull request Jan 12, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants