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

[RFC] Migrate legacy tests #1328

Closed
wants to merge 25 commits into from
Closed

Conversation

rainerborene
Copy link
Sponsor Contributor

This is just the start. More commits are coming!

@rainerborene rainerborene changed the title Migrate legacy test 4 Migrate legacy tests Oct 22, 2014
@rainerborene rainerborene changed the title Migrate legacy tests [WIP] Migrate legacy tests Oct 22, 2014
@rainerborene
Copy link
Sponsor Contributor Author

ping @justinmk

@tarruda
Copy link
Member

tarruda commented Oct 22, 2014

Nice!

@justinmk
Copy link
Member

Awesome work. Let's keep the commit message pattern for test migrations like this:

legacy tests: migrate test35

@aktau
Copy link
Contributor

aktau commented Oct 23, 2014

Smashing work indeed, looks great!

@fwalch
Copy link
Member

fwalch commented Oct 23, 2014

@rainerborene Looks great! If you want, have a look at #1287, it could help with some tests.

@rainerborene
Copy link
Sponsor Contributor Author

@tarruda Could you have a look on commits 741fe6d and 9f1dc22?

@tarruda
Copy link
Member

tarruda commented Oct 27, 2014

Looking good so far, great work!

@rainerborene
Copy link
Sponsor Contributor Author

@tarruda It's done.

@tarruda
Copy link
Member

tarruda commented Oct 27, 2014

@tarruda It's done.

If you are done with this PR, change the [WIP] tag to [RFC] so others know when it's ready for review

@rainerborene
Copy link
Sponsor Contributor Author

@tarruda Sorry. I meant, squash and commits reordering.

@tarruda
Copy link
Member

tarruda commented Oct 27, 2014

@tarruda Sorry. I meant, squash and commits reordering.

Np. feel free to leave squash/reordering to when you are done with the PR

@tarruda
Copy link
Member

tarruda commented Oct 28, 2014

This PR is already kinda big. Perhaps you want to mark as [RDY](once tests are fixed) and continue on another PR? There are some changes going on the test modules, and those can make it difficult for you to rebase later.

@elmart
Copy link
Contributor

elmart commented Oct 28, 2014

Just one comment. It doesn't seem to affect, but, shouldn't we remove deleted scripts from test Makefile's SCRIPT'S variable?

SCRIPTS := test_autoformat_join.out                                    \
           test_eval.out                                               \
                       test2.out   test3.out   test4.out   test5.out   \
           test6.out   test7.out   test8.out   test9.out   test10.out  \
           test11.out  test12.out  test13.out  test14.out  test15.out  \
                       test17.out  test18.out  test19.out  test20.out  \
           test21.out  test22.out  test23.out  test24.out  test25.out  \
           test26.out  test27.out  test28.out  test29.out  test30.out  \
           test31.out  test32.out  test33.out  test34.out  test35.out  \
           test36.out  test37.out  test38.out  test39.out  test40.out  \
           test41.out  test42.out  test43.out  test44.out  test45.out  \
           test46.out  test47.out  test48.out  test49.out              \
           test51.out  test52.out  test53.out  test54.out  test55.out  \
           test56.out  test57.out  test58.out  test59.out  test60.out  \
           test61.out  test62.out  test63.out  test64.out  test65.out  \
           test66.out  test67.out  test68.out  test69.out  test70.out  \
           test71.out  test72.out  test73.out  test74.out  test75.out  \
           test76.out  test77.out  test78.out  test79.out  test80.out  \
           test81.out  test82.out  test83.out  test84.out  test85.out  \
           test86.out  test87.out  test88.out  test89.out  test90.out  \
           test91.out  test92.out  test93.out  test94.out  test95.out  \
           test96.out  test97.out  test98.out  test99.out  test100.out \
           test101.out test102.out test103.out test104.out test105.out \
           test106.out test107.out                                     \
           test_options.out                                            \
           test_listlbr.out test_listlbr_utf8.out                      \
           test_changelist.out                                         \
           test_breakindent.out                                        \
           test_insertcount.out                                        \
           test_utf8.out

@elmart
Copy link
Contributor

elmart commented Oct 28, 2014

Also, are we sure we want to delete old tests?
I think it's great having tests rewritten in a way that's much comfortable to work with, and using those in normal workflow.
But keeping old tests and being able to run them once in a while, just to check we remain test-wise compatible to vim is also a good thing, IMO.

@tarruda
Copy link
Member

tarruda commented Oct 28, 2014

But keeping old tests and being able to run them once in a while, just to check we remain test-wise compatible to vim is also a good thing, IMO.

The test input/output is exacly the same, it would be redundant to keep a whole suite that tests the exact same thing(The converted tests are even better because they simulate a real user entering input)

@justinmk
Copy link
Member

Needs RFC before RDY.

@tarruda
Copy link
Member

tarruda commented Oct 28, 2014

Needs RFC before RDY.

👍

@rainerborene
Copy link
Sponsor Contributor Author

@tarruda Great job! 👍

@tarruda
Copy link
Member

tarruda commented Nov 4, 2014

Other than my last comment, LGTM. I will merge after you fix it.

@rainerborene
Copy link
Sponsor Contributor Author

@tarruda fixed.

tarruda added a commit that referenced this pull request Nov 4, 2014
@tarruda
Copy link
Member

tarruda commented Nov 4, 2014

👍 merged, thanks

@tarruda tarruda closed this Nov 4, 2014
lucc added a commit to lucc/neovim that referenced this pull request Jul 18, 2015
Remove legacy tests from the old makefile that were forgotten after the test
migration.  The tests are:

- test 26 migrated in da3ade6 merged in neovim#1420
- test 27 migrated in ac52d84 merged in neovim#1328
- test 43 migrated in c915958 merged in neovim#1420
- test 46 migrated in 5ea94e1 merged in neovim#1428
- test 63 migrated in a040aa9 merged in neovim#1930

Additionally the tests 71 and 72 where removed in
85338fe together with the +cryptv feature.
ghost pushed a commit that referenced this pull request Jul 19, 2015
Remove legacy tests from the old makefile that were forgotten after the test
migration.  The tests are:

- test 26 migrated in da3ade6 merged in #1420
- test 27 migrated in ac52d84 merged in #1328
- test 43 migrated in c915958 merged in #1420
- test 46 migrated in 5ea94e1 merged in #1328
- test 63 migrated in a040aa9 merged in #1930

Additionally the tests 71 and 72 where removed in
85338fe together with the +cryptv feature.
mgraczyk pushed a commit to mgraczyk/neovim that referenced this pull request Aug 10, 2015
Remove legacy tests from the old makefile that were forgotten after the test
migration.  The tests are:

- test 26 migrated in da3ade6 merged in neovim#1420
- test 27 migrated in ac52d84 merged in neovim#1328
- test 43 migrated in c915958 merged in neovim#1420
- test 46 migrated in 5ea94e1 merged in neovim#1328
- test 63 migrated in a040aa9 merged in neovim#1930

Additionally the tests 71 and 72 where removed in
85338fe together with the +cryptv feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants