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

.travis.yml: remove mingw builds from allow_failures. #3277

Merged
merged 1 commit into from Jan 5, 2018

Conversation

Projects
None yet
3 participants
@mkrautz
Member

mkrautz commented Dec 2, 2017

Our MinGW build was left stale due to #3208.

Although our current MinGW builders use Wine as their test runner
-- and were added to allow_failures exactly for that reason --
it makes more sense to get blocked from merging on build failures.
The alternative is that we actively check that the MinGW is not
broken, which doesn't happen in practice.

We obviously can't expect Wine to mirror the exact behavior on any
particular Windows version, but let's just look at the MinGW targets
in .travis.yml as a check for whether a) the MinGW build works on Linux,
and b) whether our tests pass using Wine.

@mkrautz mkrautz requested review from Kissaki, hacst and davidebeatrici Dec 2, 2017

@Kissaki

We obviously can't expect Wine to mirror the exact behavior on any
particular Windows version

But tests currently pass/tested windows expectations currently match?

but let's just look at the MinGW targets
in .travis.yml as a check

I’m a little confused by this wording. This does not refer to manually checking or otherwise checking, but just that we have the builds that can fail, right?

@Kissaki

This comment has been minimized.

Show comment
Hide comment
@Kissaki

Kissaki Dec 31, 2017

Member

But tests currently pass/tested windows expectations currently match?

travis mingw build currently fails TestServerResolver:

FAIL!  : TestServerResolver::srvCustomPort() Compared values are not the same
Member

Kissaki commented Dec 31, 2017

But tests currently pass/tested windows expectations currently match?

travis mingw build currently fails TestServerResolver:

FAIL!  : TestServerResolver::srvCustomPort() Compared values are not the same
@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Dec 31, 2017

Member

SRV resolving is failing on Wine for some reason.
I also think the resolver code could be cleaner and have better error reporting. I'll look into it.

Member

mkrautz commented Dec 31, 2017

SRV resolving is failing on Wine for some reason.
I also think the resolver code could be cleaner and have better error reporting. I'll look into it.

@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Jan 2, 2018

Member

This failure is reproducible on my local test system.

Member

mkrautz commented Jan 2, 2018

This failure is reproducible on my local test system.

@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Jan 2, 2018

Member

On Wine, SRV resolving yields

QDnsLookup::ServerFailureError 5 the server encountered an internal failure while processing the request (SERVFAIL).
Member

mkrautz commented Jan 2, 2018

On Wine, SRV resolving yields

QDnsLookup::ServerFailureError 5 the server encountered an internal failure while processing the request (SERVFAIL).
@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Jan 2, 2018

Member

I suggest we do the following:

  1. File a Wine bug for this.
  2. Add a header file with utilities that can be included in tests.
  3. Add an IsWine (name up for debate) inline function to it that detects whether we're on Wine. (Check wine_get_version in ntdll -- https://wiki.winehq.org/Developer_FAQ#How_can_I_detect_Wine.3F)
  4. Skip these tests on Wine.
Member

mkrautz commented Jan 2, 2018

I suggest we do the following:

  1. File a Wine bug for this.
  2. Add a header file with utilities that can be included in tests.
  3. Add an IsWine (name up for debate) inline function to it that detects whether we're on Wine. (Check wine_get_version in ntdll -- https://wiki.winehq.org/Developer_FAQ#How_can_I_detect_Wine.3F)
  4. Skip these tests on Wine.
@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Jan 2, 2018

Member

Reproducible exe in a zip:
TestServerResolver.zip

Member

mkrautz commented Jan 2, 2018

Reproducible exe in a zip:
TestServerResolver.zip

@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Jan 2, 2018

Member

Fails on Wine (wine-1.8.5 (Ubuntu 1.8.5-1ubuntu2)), works on Windows 10 1709.

Member

mkrautz commented Jan 2, 2018

Fails on Wine (wine-1.8.5 (Ubuntu 1.8.5-1ubuntu2)), works on Windows 10 1709.

@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz
Member

mkrautz commented Jan 4, 2018

.travis.yml: remove mingw builds from allow_failures.
Our MinGW build was left stale due to #3208.

Altough our current MinGW builders use Wine as their test runner
-- and were added to allow_failures exactly for that reason --
it makes more sense to get blocked from merging on build failures.
The alternative is that we actively *check* that the MinGW is not
broken, which doesn't happen in practice.

We obviously can't expect Wine to mirror the exact behavior on any
particular Windows version, but let's just look at the MinGW targets
in .travis.yml as a check for whether a) the MinGW build works on Linux,
and b) whether our tests pass using Wine.
@mkrautz

This comment has been minimized.

Show comment
Hide comment
@mkrautz

mkrautz Jan 5, 2018

Member

Rebased on top of #3295.

Member

mkrautz commented Jan 5, 2018

Rebased on top of #3295.

@mkrautz mkrautz merged commit 76eb586 into mumble-voip:master Jan 5, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment