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

put test output within its RUN and PASS/FAIL lines #1899

Merged
merged 2 commits into from
Aug 6, 2018

Conversation

bhcleek
Copy link
Collaborator

@bhcleek bhcleek commented Jul 25, 2018

Put test output within its RUN and PASS/FAIL lines so that it's easier to identify which test is providing the output.

For instance, the Test_Vet in lint_test.vim outputs vim-go: [vet] FAIL. Previously, this output would be output after the test results:

=== RUN  Test_Vet
--- PASS Test_Vet (0.628221s)
vim-go: calling vet...
vim-go: [vet] FAIL
ok   lint_test.vim              2.651954s / 4 tests

But now, the output will be shown within the test run:

=== RUN  Test_Vet
vim-go: calling vet...
vim-go: [vet] FAIL
--- PASS Test_Vet (0.618306s)
ok   lint_test.vim              2.598650s / 4 tests

@bhcleek
Copy link
Collaborator Author

bhcleek commented Jul 25, 2018

:messages clear wasn't added until 7.4.1735 (https://github.com/vim/vim/releases/tag/v7.4.1735), but we currently install 7.4.1689 for testing. 😢

@bhcleek bhcleek closed this Jul 25, 2018
@arp242
Copy link
Contributor

arp242 commented Jul 25, 2018

We can bump the version; also see #1847.

I think bumping the minimally required version to 7.4.2008 is reasonable.

@bhcleek
Copy link
Collaborator Author

bhcleek commented Jul 25, 2018

works for me. I'll re-open this and bump the version tonight.

Update minimum required version of Vim to 7.4.2009; this ensures that
`:messages clear` (needed for the changes to scripts/runtest) exists as
well as `execute()` (which we want for other reasons).
@bhcleek bhcleek reopened this Jul 25, 2018
@fatih
Copy link
Owner

fatih commented Aug 6, 2018

lgtm, but add a line in changelog to backwards incompatibility section

@bhcleek bhcleek merged commit f20097d into fatih:master Aug 6, 2018
@bhcleek bhcleek deleted the place-test-output branch August 6, 2018 20:39
bhcleek added a commit that referenced this pull request Aug 6, 2018
Update CHANGELOG.md for #1894, #1895, #1899, #1900, #1901, and #1905.
" release (16.04) uses.
" Version 7.4.2009 was chosen because that's greater than what the most recent Ubuntu LTS
" release (16.04) uses and has a couple of features we need (e.g. execute()
" and :message clear).
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this comment is no longer true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Which part isn't true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, you mean that 16.04 isn't the most recent version of LTS anymore?

Copy link
Contributor

@arp242 arp242 Aug 6, 2018

Choose a reason for hiding this comment

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

Yeah, and that the minimal version is chosen because it's what in the most recent Ubuntu LTS.

@benjamin-thomas
Copy link

A pretty annoying change IMHO.

For such a cosmetic "bug" I would think most users wouldn't care about (but would care about broken code completion for example).

Anyways as an Ubuntu 16.04 users I would not expect having to upgrade my OS until april 2021.

So as a user I have the choice:

  • to upgrade my OS, for a vim plugin
  • compile vim, will probably be a pain
  • add that variable to my config, and expect unreliable behavior from vim-go from now on.

@arp242
Copy link
Contributor

arp242 commented Sep 27, 2018

Anyways as an Ubuntu 16.04 users I would not expect having to upgrade my OS until april 2021.

I'm sorry @benjamin-thomas, but you can't expect people to keep supporting a Vim version that's years old. Ubuntu's policy is their responsibility, not ours. 7.4.2008 is already more than two years old. Two years is a very reasonable time period to support software.

Installer a newer Vim version on Ubuntu isn't very hard: https://vi.stackexchange.com/q/10817/51

For such a cosmetic "bug" I would think most users wouldn't care about (but would care about broken code completion for example).

It's not about this "cosmetic bug", it's about allowing vim-go to use newer and more convenient Vim features throughout. There have been a lot of changes to Vim in the last few years, and supporting older Vim versions is a bit of a pain.
We discussed changing the version before, and just happened to be that in the discussion for this bug we updated the Vim version.

@benjamin-thomas
Copy link

Please don't take it personally, I've always loved vim-go.

And I don't expect anything from an opensource project.

But I'll merely state that forcing an upgrade on a stable OS for no obvious reason is not reasonable at all IMO, and disappointing.

And I find this new support policy to be in sharp contrast with the go mentality of having long stable tools.

This is not a technical hurdle for me, more of a nuisance than anything.

My 2 cents...

@arp242
Copy link
Contributor

arp242 commented Sep 27, 2018

And I find this new support policy to be in sharp contrast with the go mentality of having long stable tools.

Go releases aren't supported for 2+ years. There are plenty of cases where you need a minimum Go version to run something (e.g. using new sort API, subtests, loads of other stuff that was introduced fairly recent, etc.)

Will you keep writing Go programs with Go 1.6 that Ubuntu 16.4 ships with until April 2021? I would hope not.

@benjamin-thomas
Copy link

The go upgrade path is painless, since you just untar the latest release when you feel like it, and you're good to go, that's what I was trying to convey.

I can confirm compiling vim was a chore, and I don't want to rely on some random dude's PPA.

At that point, updating my editor plugin becomes more burdensome than upgrading the underlying tool chain.

I can only assume you're running a Mac, brew install and probably don't care?

Anyways I've been using vim for 10 years and never faced such a requirement from a vim plugin, take it for what you will.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants