[Merged by Bors] - run some examples on CI using swiftshader #1826
Conversation
alice-i-cecile
added
A-Build-System
|
Should we be mocking input for the game examples as well? It would be nice to catch regressions there too. |
|
This could be a nice later evolution, adding for example to the debug config file a list of |
mockersf
changed the title
| @@ -0,0 +1,3 @@ | |||
| ( | |||
There was a problem hiding this comment.
I would claim that specifying which example to run in the config would be wise, which would be useful to test diverging paths once we accept input.
There was a problem hiding this comment.
after thinking about this, I would prefer to keep the example name outside and instead allow file like breakout-1.ron, breakout-2.ron, ...
having the name inside means that:
- to get it from CI we have to parse the ron in the GitHub action
- once the example started, it is not useful anymore
what do you think?
There was a problem hiding this comment.
Hmm for now I'm cool with using it as-is. Maybe long term we can write a rust program that orchestrates the tests / parses + runs the ron files. Then the github action could just run the rust program.
Embedding the app file name it in the test name creates "collision problems". Ex: what if we create a breakout-1.rs example?
|
|
||
| - name: Setup swiftshader | ||
| run: | | ||
| wget https://github.com/qarmin/gtk_library_store/releases/download/3.24.0/swiftshader.zip; |
There was a problem hiding this comment.
Can we trust this? The repo doesn't have any readme or sources and doesn't tell how it was built. In addition there was exactly a single release. I think it would be better if we made a repo in the bevyengine org that uses CI to build swiftshader and download this pre-compiled version.
There was a problem hiding this comment.
origin from this download: godotengine/godot#38428 (comment)
I think we can trust it, but I agree it would be better to have a version owned by Bevy. I can setup a repo once Cart creates it
There was a problem hiding this comment.
Hmm as long as we aren't passing in secrets, do we need to trust it? Whats the worst case scenario here?
There was a problem hiding this comment.
Nothing really bad, mostly breaking our CI by changing/removing the download. Also we don't have the possibility to update if we need to
There was a problem hiding this comment.
I believe the github token passed for branch pushes (eg bors r+) has write access as far as I know. I don't know if it is possible to access it some way if it isn't explicitly used in the workflow file though.
There was a problem hiding this comment.
Yeah the github token definitely has write access. I hope workflows only have access if it is explicitly passed in to a command in the workflow file. Otherwise we should reconsider our use of things like superlinter
There was a problem hiding this comment.
the ci during branch push is not triggered by Bors but by the event push on branch staging, I would consider it a big bug in GitHub Actions if Bors token leaked here
There was a problem hiding this comment.
You need write permission to push to a branch. If the github token is explicitly referenced in the workflow file it is definitively possible to use it to write things.
There was a problem hiding this comment.
I've dug into this a bit deeper. Everything I've read indicates that secrets are not accessible by a workflow container unless explicitly passed in. I don't have the bandwidth to do a full pen-test here, so I'll just trust what I've read (and the knowledge that if it didn't behave the way we've discussed, there would be very serious security implications that would completely erode people's trust in github).
There was a problem hiding this comment.
we discussed this a bit today, secrets are not accessible during pr tests, and the GITHUB_TOKEN, should someone add it in an action, would be a readonly token
.github/workflows/ci.yml
Outdated
|
|
||
| - name: Build bevy | ||
| run: | | ||
| cargo build --no-default-features --features "bevy_dynamic_plugin,bevy_gilrs,bevy_gltf,bevy_wgpu,bevy_winit,render,png,hdr,x11,bevy_debug" |
There was a problem hiding this comment.
How long will the CI job take when compiling in release mode?
There was a problem hiding this comment.
it was a few minutes longer overall in release: https://github.com/mockersf/bevy/runs/2271841611?check_suite_focus=true
|
I think this is a solid first step into automated "end-to-end" bevy tests. I sort of expect the ron-config to eventually be overly limiting, but I think it works perfectly for the current "does example run without crashing" style of test. For more complicated tests we'll likely want full control of the app. It would be interesting to explore ways to "inject" code-driven tests into pre-written apps (ex: click this thing, wait 10 seconds, then run this system that checks the value of a resource or validates pixels rendered to the screen). Maybe something like what we used to do for android example: // only works for single file examples
include!("../3d/3d_scene.rs");
// enable a `bevy_app` feature that somehow injects this plugin into the App above? maybe not possible
struct TestPlugin;
impl Plugin for TestPlugin {
// add test systems/resources here
} Or alternatively we just create new Apps/files for the more complicated tests. |
mockersf
force-pushed
the
ci-run-examples
branch
from
4c0e569
to
509ee63
Compare
|
bors r+ |
From suggestion from Godot workflows: #1730 (comment) * Add a feature `bevy_debug` that will make Bevy read a debug config file to setup some debug systems * Currently, only one that will exit after x frames * Could add option to dump screen to image file once that's possible * Add a job in CI workflow that will run a few examples using [`swiftshader`](https://github.com/google/swiftshader) * This job takes around 13 minutes, so doesn't add to global CI duration |example|number of frames|duration| |-|-|-| |`alien_cake_addict`|300|1:50| |`breakout`|1800|0:44| |`contributors`|1800|0:43| |`load_gltf`|300|2:37| |`scene`|1800|0:44|
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
|
Just realized that bors doesn't run the new tests 😄 @mockersf can you add the new job name to this file? |
oh missed that file :( |
followup on #1826 (comment): adding the new job to Bors
From suggestion from Godot workflows: bevyengine#1730 (comment) * Add a feature `bevy_debug` that will make Bevy read a debug config file to setup some debug systems * Currently, only one that will exit after x frames * Could add option to dump screen to image file once that's possible * Add a job in CI workflow that will run a few examples using [`swiftshader`](https://github.com/google/swiftshader) * This job takes around 13 minutes, so doesn't add to global CI duration |example|number of frames|duration| |-|-|-| |`alien_cake_addict`|300|1:50| |`breakout`|1800|0:44| |`contributors`|1800|0:43| |`load_gltf`|300|2:37| |`scene`|1800|0:44|
followup on bevyengine#1826 (comment): adding the new job to Bors
From suggestion from Godot workflows: #1730 (comment)
bevy_debugthat will make Bevy read a debug config file to setup some debug systemsswiftshaderalien_cake_addictbreakoutcontributorsload_gltfscene