Skip to content

Appveyor CI changes for CMake#2332

Merged
Holzhaus merged 28 commits intomixxxdj:masterfrom
Holzhaus:cmake-ci
Feb 11, 2020
Merged

Appveyor CI changes for CMake#2332
Holzhaus merged 28 commits intomixxxdj:masterfrom
Holzhaus:cmake-ci

Conversation

@Holzhaus
Copy link
Copy Markdown
Member

These are the CI changes that were removed from #2280.

@uklotzde uklotzde added this to the 2.4.0 milestone Oct 27, 2019
@uklotzde uklotzde changed the title CI changes for CMake [WIP] CI changes for CMake Oct 27, 2019
@uklotzde
Copy link
Copy Markdown
Contributor

Requires #2280 as a prerequisite.

@PatrickLang
Copy link
Copy Markdown

@Holzhaus - what's the latest on this? Anything I can help with or test out? I work on Windows + Linux and am happy to do some check on the Windows side if that's helpful.

@Holzhaus
Copy link
Copy Markdown
Member Author

Holzhaus commented Dec 2, 2019

@Holzhaus - what's the latest on this? Anything I can help with or test out? I work on Windows + Linux and am happy to do some check on the Windows side if that's helpful.

Are you referring to CMake support? This has already landed in master and will be part of the 2.3 release (although scons will remain the official way to build Mixxx in 2.3). These CI changes will only become relevant for Mixxx 2.4.

However, at least for Travis CI we could just add 2 more jobs that build Mixxx via CMake, too. I don't see any downside to this IMHO. I'll look into it when #2375 has been merged.

@uklotzde
Copy link
Copy Markdown
Contributor

Does this PR become obsolete with #2392?

@Holzhaus
Copy link
Copy Markdown
Member Author

No, we still need the Appveyor changes.

@Holzhaus Holzhaus changed the title [WIP] CI changes for CMake Appveyor CI changes for CMake Feb 6, 2020
@Holzhaus
Copy link
Copy Markdown
Member Author

Holzhaus commented Feb 6, 2020

After merging #2490 this can replace the SCons based CI builds on Appveyor. It's not ideal to do this before dropping SCons, but I'd argue that this is preferable to all the timeouts that happen right now.

@uklotzde
Copy link
Copy Markdown
Contributor

uklotzde commented Feb 8, 2020

I support the decision to introduce this workaround! Keeping the constantly failing SCons CI build is useless.

Afterwards we need to keep an eye on our own build servers that still use SCons for the official builds. A fair deal.

@Holzhaus
Copy link
Copy Markdown
Member Author

What do the other devs think? Should we merge this now to get rid of the timeouts? @daschuer @Be-ing @rryan etc.?

@daschuer
Copy link
Copy Markdown
Member

Yes, failing CI is anoying. However without a scons CI, we have no early warning that something goes wrong.
I would support to drop scons now and than merge this. Or can we keep the scons Target in addition?

Why are disabled tests reported as failing? What happens if s test actually fails?

@Holzhaus
Copy link
Copy Markdown
Member Author

Holzhaus commented Feb 11, 2020

We'd still have it on Travis.

@uklotzde
Copy link
Copy Markdown
Contributor

We'd still have ir on Travis.

We have Travis for Linux and macOS and our own build server for Windows. There is a low chance to break the Windows build with an SCons change. But the SCons build should not be updated very often in the future.

@daschuer
Copy link
Copy Markdown
Member

Ah OK, so we can safely merge this.

Is there a chance to not report the disabled tests as failing?
What happens if a test actually fails. Are we still green than?

@Holzhaus
Copy link
Copy Markdown
Member Author

Is there a chance to not report the disabled tests as failing?

Apparently the XSLT that converts the ctest results to JUnit XML did not support skipped tests. I added support in 2334558.

What happens if a test actually fails. Are we still green than?

No, because ctest should terminate with a non-zero exit status.

@Holzhaus Holzhaus merged commit 8f265f3 into mixxxdj:master Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants