Fold Travis CI output by component #22461
Conversation
2d59800 to
eebdb9a
Compare
f50048d to
da9bd62
Compare
.travis.yml
Outdated
| @@ -40,6 +40,10 @@ cache: | |||
| services: mongodb | |||
|
|
|||
| before_install: | |||
| - fold_start () { echo "travis_fold:start:$(echo $1 | tr / .)"; echo -e "\\e[1;34m$1\\e[0m"; } | |||
| - fold_end () { echo "travis_fold:end:$(echo $1 | tr / .)"; } | |||
| - fold_wrap () { $T=$1; shift; fold_wrap_start $T; sh -c "$*" && fold_wrap_end $T; } | |||
There was a problem hiding this comment.
are you sure about fold_wrap_start and fold_wrap_end ? shouldn't it be fold_start and fold_end instead ?
And fold_wrap_end will not be called if the command is failing. Is it expected ?
.travis.yml
Outdated
| - if [[ ! $skip && $deps ]]; then export SYMFONY_DEPRECATIONS_HELPER=weak; fi | ||
| - if [[ ! $skip && $deps ]]; then mv composer.json.phpunit composer.json; fi | ||
| - if [[ ! $skip ]]; then composer update --no-suggest; fi | ||
| - if [[ ! $skip ]]; then $COMPOSER_UP; fi |
There was a problem hiding this comment.
is it necessary here ? It is already folded by Travis
.travis.yml
Outdated
| - | | ||
| run_tests () { | ||
| set -e | ||
| if [[ ! $PHP = hhvm* ]]; then fold_wrap phpinfo php -i; else fold_wrap phpinfo hhvm --php -r 'print_r($_SERVER);print_r(ini_get_all());'; fi |
dac6d79 to
8821db0
Compare
|
also embeds a rewrite of the .travis.yml to make it more readable hopefully |
|
It's really more readable like this! 👍 |
|
@nicolas-grekas amazing work! The Travis output looks gorgeous and it's useful again!! Just asking: should we use "green" instead of "blue" for tests that pass? See the before/after comparison of the output:
|
|
@javiereguiluz As it's me that have done this color change, I can response you. Outputs of Composer and PHPUnit use a lot of green color. Highlight title fold with an other color looks me fine for readability. |
|
@maidmaid my reasoning is: if I see some fold green I think "great, everything is working; no need to open it". If I see a blue fold I think: "Mmmm, a fold ... let's open it to see if it worked or failed". |
8e709f9 to
a936ffd
Compare
|
Here is using "travis green": https://travis-ci.org/symfony/symfony/jobs/223214166 |
| PHP=$TRAVIS_PHP_VERSION | ||
| [ -d ~/.composer ] || mkdir ~/.composer | ||
| cp .composer/* ~/.composer/ | ||
| export PHPUNIT=$(readlink -f ./phpunit) |
There was a problem hiding this comment.
are you sure that failures in these commands actually make Travis mark the build as errored ? the main Travis build script does not run with -e (it would break their own reporting). I think they would report an error only for the last statement of your big string
There was a problem hiding this comment.
set -e now moved at the beginning of the file
There was a problem hiding this comment.
There was a problem hiding this comment.
this is a bad idea, as it will put the whole Travis script in -e mode, breaking Travis features (see that your build output does not have the Travis failure report at the end). This is also why the Travis docs says we should never use exit in a script command.
If you want to use -e, you have to make it in a script executed through sh, not directly in the main commands.
There was a problem hiding this comment.
indeed, fixed by chaining commands with &&
There was a problem hiding this comment.
we don't need this for all commands, the missing ones are on purpose
fbaeada to
f052f18
Compare
|
I love this, it looks really cool 😄🤘 But should tests with skipped tests be marked yellow instead of green? |
|
@sstok we don't know what a skipped test is from this pov: we just run a command and color it based on its exit code. I don't think we should make the code more specific about what is running (phpunit or whatever) |
|
OK, no problem 😉 |
adf606d to
00260d2
Compare
|
I reverted to streaming the output instead of buffering it so that we always have a chance to get the output (buffering has the potential to swallow useful info when a bad failure happens). This means the green/red coloring of the folding title is gone, thus blue is used. Replaced by the previously existing green/red OK/KO line at the end of each folded block. |
00260d2 to
cda6087
Compare
| [[ $deps = high && ${SYMFONY_VERSION%.*} != $(git show $(git ls-remote --heads | grep -FA1 /$SYMFONY_VERSION | tail -n 1):composer.json | grep '^ *"dev-master". *"[1-9]' | grep -o '[0-9]*' | head -n 1) ]] && LEGACY=,legacy | ||
|
|
||
| export COMPOSER_ROOT_VERSION=$SYMFONY_VERSION.x-dev | ||
| if [[ ! $skip && $deps ]]; then mv composer.json.phpunit composer.json; fi |
There was a problem hiding this comment.
do we actually need this one ? our PHPUnit wrapper already handles the installation of the bridge, not need to force again the usage of the one requiring it
There was a problem hiding this comment.
this allows using the latest version of the bridge (maybe patched by the current PR), with a previous version of the code base (eg on deps=high on master branch where we checkout 3.2)
so yes we need it
cda6087 to
d5db6a1
Compare
d5db6a1 to
cf87678
Compare
|
Just pushed a last tweak: |
|
Thank you @nicolas-grekas. |
…ekas) This PR was merged into the 2.7 branch. Discussion ---------- Fold Travis CI output by component | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Trying some tweaks on top of #22252 Commits ------- cf87678 Make .travis.yml more readable 7a9b086 Fold Travis CI output by component
Trying some tweaks on top of #22252