Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Upgrading utf8 comparisons and sha256 tests#1797

Closed
pschuh wants to merge 2 commits intovim:mastervim/vim:masterfrom
pschuh:test-upgradeCopy head branch name to clipboard
Closed

Upgrading utf8 comparisons and sha256 tests#1797
pschuh wants to merge 2 commits intovim:mastervim/vim:masterfrom
pschuh:test-upgradeCopy head branch name to clipboard

Conversation

@pschuh
Copy link

@pschuh pschuh commented Jun 26, 2017

test82.in -> test_utf8_comparisons.vim
test90.in -> test_sha256.vim

@chrisbra
Copy link
Member

oh wow, that are a lot of changes. Can you please rename the new numeric tests like test24.vim to a better name, e.g. test23.vim could be named test_ex_edit.vim, test24.vim -> test_regex_backslash.vim, test26.vim -> test_exe_while_if.vim, test67.vim -> test_exists.vim, test75.vim -> test_maparg.vim, test82.vim -> test_mbyte_strnicmp.vim, test90.vim -> test_sha256vim, test97.vim -> test_glob.vim.
My suggestions might not be the best, however the names should be descriptive so use whatever you think fits best. And it should be sorted in the Makefile. Thanks.

Copy link
Member

@dpelle dpelle left a comment

Choose a reason for hiding this comment

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

Thanks!
Consider creating several independent
pull requests rather than a big one.

let mod = ":t"
if a:tab > 0 && a:tab != tabpagenr()
let tab_changed = 1
exec "tabnext " . a:tab
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit hard to read without indenting the if and else blocks.

return bufname . ' ' . dirname . ' ' . lflag
endfunction

" Do all test in a separate window to avoid E211 when we recursively
Copy link
Member

Choose a reason for hiding this comment

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

all test → all tests

@codecov-io
Copy link

Codecov Report

Merging #1797 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1797      +/-   ##
==========================================
- Coverage   75.11%   75.07%   -0.04%     
==========================================
  Files          76       76              
  Lines      125109   125150      +41     
==========================================
- Hits        93978    93960      -18     
- Misses      31131    31190      +59
Impacted Files Coverage Δ
src/if_xcmdsrv.c 83.33% <0%> (-2.6%) ⬇️
src/mbyte.c 60.35% <0%> (-1.91%) ⬇️
src/os_unix.c 58.28% <0%> (-0.35%) ⬇️
src/channel.c 83.67% <0%> (-0.24%) ⬇️
src/window.c 81.62% <0%> (-0.2%) ⬇️
src/evalfunc.c 81.5% <0%> (-0.13%) ⬇️
src/eval.c 80.74% <0%> (-0.08%) ⬇️
src/gui_gtk_x11.c 47.39% <0%> (-0.06%) ⬇️
src/gui.c 45.58% <0%> (ø) ⬆️
src/if_py_both.h 76.02% <0%> (+0.7%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41cc038...9a114aa. Read the comment docs.

@pschuh pschuh changed the title Upgrading sundry tests to new test format. Upgrading utf8 comparisons and sha256 tests Jun 27, 2017
@brammool brammool closed this in 28b2382 Jun 27, 2017
@brammool
Copy link
Contributor

brammool commented Jun 27, 2017 via email

dpelle pushed a commit to dpelle/vim that referenced this pull request Jul 31, 2017
Problem:    Old style tests are not nice.
Solution:   Turn two tests into new style. (pschuh, closes vim#1797)
justinmk added a commit to justinmk/neovim that referenced this pull request Feb 11, 2018
Problem:    Old style tests are not nice.
Solution:   Turn two tests into new style. (pschuh, closes vim/vim#1797)

vim/vim@28b2382
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.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.