treewide: enable working, fix fixable, disable broken tests #44825
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GrahamcOfBorg
added
6.topic: GNOME
matthewbauer
reviewed
Aug 9, 2018
| # sed -i -e 's@dbus/[^ ]*\.\(test\|vala\)@@g' tests/Makefile.am | ||
| # ''; | ||
| # | ||
| # checkInputs = [ automake_1_15 ]; |
There was a problem hiding this comment.
This looks like we should just leave off it isn't working.
matthewbauer
reviewed
Aug 9, 2018
| postPatch = '' | ||
| for a in test/Makefile.in test/format_test/format_checks.sh.in ; do | ||
| substituteInPlace $a \ | ||
| --replace /bin/bash ${bash}/bin/bash |
There was a problem hiding this comment.
I think it's considered more portable to do stdenv.shell instad of ${bash}/bin/bash.
matthewbauer
reviewed
Aug 9, 2018
| @@ -26,9 +25,7 @@ in stdenv.mkDerivation rec { | |||
|
|
|||
| propagatedBuildInputs = [ zlib ]; | |||
|
|
|||
| # it's hard to cross-run tests and some check programs didn't compile anyway | |||
| makeFlags = stdenv.lib.optional (!doCheck) "check_PROGRAMS="; | |||
There was a problem hiding this comment.
This might still be needed when stdenv.hostPlatform /= stdenv.buildPlatform;
jtojnar
reviewed
Aug 9, 2018
| @@ -22,14 +22,11 @@ stdenv.mkDerivation rec { | |||
| pango gcr gdk_pixbuf atk p11-kit | |||
| ]; | |||
|
|
|||
| # In 3.20.1, tests do not support Python 3 | |||
| checkInputs = [ dbus python2 ]; | |||
There was a problem hiding this comment.
Could you keep the comment?
matthewbauer
reviewed
Aug 9, 2018
pkgs/tools/misc/parted/default.nix
Outdated
|
|
||
| # The `t0400-loop-clobber-infloop.sh' test wants `mkswap'. | ||
| preCheck = '' | ||
| export PATH=${utillinux}/sbin:$PATH |
There was a problem hiding this comment.
This should no longer be needed, sbin is a symlink to bin.
matthewbauer
reviewed
Aug 9, 2018
oxij
force-pushed
the
docheck/continues-in-a-big-way
branch
from
a3dc51c
to
89efaf6
Compare
|
I rebased the fixes to comments and added all the required patches to build all the packages this touches with `doCheckByDefault` enabled (unless they conflicted, those I will respin later). I also built all of said packages.
So, in short, I think this should be ready to merge, but even if you see no issues, please wait till noon UTC tomorrow, just in case.
@matthewbauer
This looks like we should just leave off it isn't working.
Yep, removed.
I think it's considered more portable to do ```stdenv.shell``` instad of ```${bash}/bin/bash.
Done.
This might still be needed when stdenv.hostPlatform /= stdenv.buildPlatform;
Nope, before this `doCheck` was set to `true` so that code was never ever run.
@jtojnar
Could you keep the comment?
Done.
The original version is kept as `docheck/continues-in-a-big-way-v1` branch in my repo for `diff`ing.
|
matthewbauer
approved these changes
Aug 10, 2018
|
In "Motivation for this change", can you please describe the actual motivation rather than point to different PRs/issues? For example, this PR's motivation refers to #39464, which helpfully is entitled "stdenv: implement most of #33599" and whose motivation says "see #39463". That's not very helpful with a PR that causes an enormous amount of code churn (127 files changed). |
Since it was used before, let's keep using it.
oxij
force-pushed
the
docheck/continues-in-a-big-way
branch
from
259f9c1
to
35c9435
Compare
|
I looked over this with a fresh head and could find only several minor OCD things.
Old versions are kept as `docheck/continues-in-a-big-way-v2` (the version before today) and `docheck/continues-in-a-big-way-v2-ocd` (the version with ocd spelled out) for diffing. I squashed OCD patches into the current version.
It merges with staging, it evaluates, everything compiles. LGTM. It's perfect. Let's merge.
|
|
ping @matthewbauer @Ericson2314 @7c6f434c
Could somebody please merge this? I think this can only get worse, not better, with coming conflicts.
|
|
Seems fine, and with all the semi-auto-updates we need the tests eventually enabled… |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
#33599 ("Run check phase by default"). This is yet another^4 (v2) followup to #39464 ("stdenv: implement most of #33599").
Things done
doCheckByDefaultset, so I think it should work sane enough on top of Nixpkgs too.Expectations