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: move BSD testing from sourcehut to Cirrus CI #19616

Merged
merged 5 commits into from
Sep 8, 2022

Conversation

dundargoc
Copy link
Member

@dundargoc dundargoc commented Aug 1, 2022

dispatch.sr.ht is being deprecated, meaning that using sourcehut CI
won't be possible (see #19609).
Since Github Actions doesn't provide any BSD runners an external service
is required and Cirrus CI seems like a good replacement for sourcehut.

Initially experimented with using FreeBSD and OpenBSD virtual machines
in GitHub Actions, but Cirrus has been a much better fit with better
performance, logs and overall experience.

Failing tests are automatically skipped on FreeBSD regardless it it's on
CI or not. Ideally these tests should only be skipped in CI with the
help of isCI helper function. Unfortunately, the tests don't recognize
the environment variable CIRRUS_CI even if it's set manually. This
workaround is good enough for the time being, but we might want to only
skip tests when using the CI (or even better, fix the failing tests).

Closes: #19609

Reminder for myself

Dispatch will shut down 2022-10-01, this will need to be merged by then.

@github-actions github-actions bot added the ci automation for build, test, and release label Aug 1, 2022
@clason

This comment was marked as outdated.

@dundargoc

This comment was marked as outdated.

@justinmk
Copy link
Member

justinmk commented Aug 2, 2022

Ok, but can we couple this with https://github.com/vmactions (see #19609 )? For BSD we at least want to check that builds work (library headers are available, etc.), if not the full test suite.

@dundargoc
Copy link
Member Author

@justinmk ok, I've gotten freebsd to work mostly. There are some tests somewhere that fail which I'll look into, but if you want to see how it will looks overall: https://github.com/neovim/neovim/runs/7638940566?check_suite_focus=true

Initial impressions of vmactions:

Pros:

  • It's easy to set up
  • It's on github, which means it plays nicely with github-features such as "restart failed jobs"

Cons:

  • The vms have the performance of a potato
  • It requires macos as the runner, which github is stingy with. This and because the runs take a long time means that this could create huge CI queues.
  • Because this needs to be run in a single step, all of the logs must be loaded at once, making the github UI a no-go. We'll need to download the logs if we'll want to debug this.

@justinmk
Copy link
Member

justinmk commented Aug 3, 2022

  • The vms have the performance of a potato

Let's only build, then. Don't run the tests.

  • It requires macos as the runner, which github is stingy with. This and because the runs take a long time means that this could create huge CI queues.

🤦 This is more of a problem.

@dundargoc dundargoc force-pushed the ci/remove-sourcehut branch 4 times, most recently from b12053c to dae5b48 Compare August 6, 2022 16:24
@clason
Copy link
Member

clason commented Aug 6, 2022

Let's only build, then. Don't run the tests.

Hmmm, we recently (with the LibUV CMake PR) had a situation where the build was successful at the face of it but failed as soon as a functionaltest was run.

Maybe we can define a (really) minimal "core" testsuite that exercises some very basic Lua/LibUV functionality? E.g., pick a single suitable TEST_FILE and just run with that?

(That would also be a candidate to run on draft PRs instead of the full suite, relieving CI pressure.)

@dundargoc
Copy link
Member Author

Maybe we can define a (really) minimal "core" testsuite that exercises some very basic Lua/LibUV functionality? E.g., pick a single suitable TEST_FILE and just run with that?

Hmm, yeah, I like this idea. Just as a quick sanity check.

I'm also wondering if we should pick one BSD distro as well. That is instead of checking if 2 BSDs build successfully and pass a few tests, we test one BSD and see if they pass a lot/most/all tests? I don't have good insight into the differences but both have "BSD" in their name so they can't be that different, right? :)

@clason
Copy link
Member

clason commented Aug 6, 2022

oldtest is a) just as slow and b) doesn't exercise Lua...

Maybe make functionaltest TEST_FILE=test/functional/lua/vim_spec.lua? (One of the bigger tests, but arguably the most important one.) Or api_spec.lua? Or loop_spec.lua (would target libuv/luv directly)?

@dundargoc dundargoc force-pushed the ci/remove-sourcehut branch 3 times, most recently from d1a8ed0 to 232234f Compare August 7, 2022 10:49
@clason
Copy link
Member

clason commented Aug 8, 2022

Looks like some (terminal) tests really don't like the VM...

@dundargoc dundargoc marked this pull request as ready for review August 8, 2022 15:16
@dundargoc
Copy link
Member Author

dundargoc commented Aug 8, 2022

@justinmk @jamessan I'd say this is pretty much finished.

Opted to only use openbsd as freebsd takes way too long for some reason. Only run the tests in functional/lua since 1. they don't fail and 2. so we don't risk CI queues if we start small due to the macos restrictions.

We can always adjust/add more tests if we notice that our CI budget allows it.

@dundargoc dundargoc changed the title ci: remove sourcehut-specific files and code ci: relocate our BSD testing to github Aug 8, 2022
@jamessan
Copy link
Member

jamessan commented Aug 8, 2022

https://github.com/cross-platform-actions/action uses a different hypervisor. I wonder how its performance compares to vmactions.

@clason
Copy link
Member

clason commented Aug 9, 2022

https://github.com/cross-platform-actions/action uses a different hypervisor. I wonder how its performance compares to vmactions.

Also runs on Linux, which is a definite plus in itself.

@dundargoc dundargoc marked this pull request as draft August 9, 2022 12:51
@github-actions github-actions bot removed the request for review from jamessan August 9, 2022 12:52
@dundargoc dundargoc force-pushed the ci/remove-sourcehut branch 9 times, most recently from dd266a4 to a251f68 Compare September 3, 2022 10:15
dispatch.sr.ht is being deprecated, meaning that using sourcehut CI
won't be possible (see neovim#19609).
Since Github Actions doesn't provide any BSD runners an external service
is required and Cirrus CI seems like a good replacement for sourcehut.

Initially experimented with using FreeBSD and OpenBSD virtual machines
in GitHub Actions, but Cirrus has been a much better fit with better
performance, logs and overall experience.

Failing tests are automatically skipped on FreeBSD regardless it it's on
CI or not. Ideally these tests should only be skipped in CI with the
help of `isCI` helper function. Unfortunately, the tests don't recognize
the environment variable CIRRUS_CI even if it's set manually. This
workaround is good enough for the time being, but we might want to only
skip tests when using the CI (or even better, fix the failing tests).

Closes: neovim#19609
@dundargoc
Copy link
Member Author

dundargoc commented Sep 4, 2022

I can't get the cirrus caching to work. Let's try without any caching since it's sufficiently fast enough anyway.

@dundargoc dundargoc marked this pull request as ready for review September 4, 2022 10:45
@clason
Copy link
Member

clason commented Sep 8, 2022

@jamessan @justinmk green light?

@clason clason mentioned this pull request Sep 8, 2022
@clason clason added this to the 0.8 milestone Sep 8, 2022
@@ -78,6 +79,9 @@ describe('notify', function()
end)

it('cancels stale events on channel close', function()
if uname() == 'freebsd' then
Copy link
Member

Choose a reason for hiding this comment

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

TODO / off topic: could make is_os('...bsd') work

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, yeah could work. I'm not against it, but let's save it for a future discussion like you said.

Copy link
Member

@justinmk justinmk left a comment

Choose a reason for hiding this comment

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

looks great. thank you for this @dundargoc

CONTRIBUTING.md Outdated Show resolved Hide resolved
@justinmk justinmk merged commit 2d6735d into neovim:master Sep 8, 2022
@dundargoc dundargoc deleted the ci/remove-sourcehut branch September 14, 2022 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci automation for build, test, and release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

migrate sourcehut builds?
4 participants