fix: Stabilise the CI #45
Conversation
|
this PR is based on - #44 |
Before this change, the bundle is dependent on the VM mount. We want to not using mounting in out VMs in the future to stablise the CI, so removing mount usage by copying the bundle to the VM. Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
| } | ||
| defer func() { _ = sim.Stop() }() | ||
| t.Cleanup(func() { | ||
| _, _, _ = vm.RunCommand("pkill", "-f", "remoteproc-simulator") |
There was a problem hiding this comment.
Why is this pkill required?
There was a problem hiding this comment.
because sim.Stop() only stops the limactl command in the host side. it wouldn't stop the process inside of the VM. the current structure also doesn't expose the VM inside the sim, so we can only stop it here. If you prefer that simulator has access to the VM besides from the simulator binary run, that would require bigger changes
| "--annotation", fmt.Sprintf("remoteproc.name=%s", "other-processor"), | ||
| imageName) | ||
| assert.Error(t, err) | ||
| assert.Contains(t, stderr, "remote processor other-processor does not exist, available remote processors: a-processor") |
There was a problem hiding this comment.
Why is this change required?
There was a problem hiding this comment.
I think it's safer this way in case of another file synchronisation delay. If the remoteproc-simulator kill of the previous test was not complete at this point, the previous test's remoteproc name would show here as error message included in the available remote processor list. If we keep the assertion this way, the test doesn't require the error string to ONLY have a-processor
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
Signed-off-by: Yejin Seo <[email protected]>
| ) | ||
|
|
||
| downloader := exec.Command("curl", "-L", "-o", filepath.Join(binOutDir, "simulator.tar.gz"), artifactURL) | ||
| downloader.Env = os.Environ() |
There was a problem hiding this comment.
Is this and line 95 actually necessary? What environment variables do we need?
| } | ||
| defer func() { _ = sim.Stop() }() | ||
| t.Cleanup(func() { | ||
| _, _, _ = vm.RunCommand("pkill", "-f", "remoteproc-simulator") |
There was a problem hiding this comment.
This should definitely be inside the sim.Stop() function. This does mean you'll need to store the vm on the Simulator struct, but it's the only way to have a coherent abstraction.
Additionally, never discard errors like this - we have no reason to assume this command should fail, so if it does fail we should at minimum log it so we can debug why the program has violated our expectation.
Changes
Checklist