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

test: Improve minimal init, test launching #616

Merged

Conversation

PriceHiller
Copy link
Contributor

@PriceHiller PriceHiller commented Sep 25, 2023

Hey there! This PR "luafies" the tests and removes the need to git clone test dependencies ahead of time by doing the clone process in the minimal init.

  • There is no longer a need to git clone any test dependencies, instead the minimal init handles that for you.
  • The minimal init cleans up after itself as much as is reasonable. This ensures previous runs have no impact on newer runs
    • This only works on Mac & Linux as it uses rm -rf to clean up after itself. I know there's a bit of a push to get better windows support. I noticed the issues and I think I may have a solution in another PR. If the rm -rf doesn't run it's not a big deal for now.
  • I removed the tesfile command and instead allowed a testfile to be specified for the make test command and if one is not found it fallbacks to tests/plenary
    • So now to launch a specific file you'd do FILE=./tests/plenary/util_spec.lua make test for instance 😉.

I know there weren't any open issues for this, but I found it slightly annoying to have to run git clone before doing any testing. If this PR is undesired feel free to reject it.

@PriceHiller
Copy link
Contributor Author

I see that the tests for Neovim 0.8.3 failed, investigating currently as to why.

@PriceHiller
Copy link
Contributor Author

I am having difficulties replicating the test failure for 0.8.3. After installing Neovim from 0.8.3 my tests are passing locally.

@PriceHiller
Copy link
Contributor Author

Seems good now, the last change was only because I caught some caching in the workflow that may have been missed prior to that commit with my changes.

Not sure why 0.8.3 is working now though 🤷, it should be the same stuff

Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This looks great!
I think we can fix the Windows support (haven't tested it though) with the below comment.

.gitignore Outdated Show resolved Hide resolved
tests/minimal_init.lua Outdated Show resolved Hide resolved
Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

Sorry for going back and forth. Just few more fixes/changes.

.gitignore Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@PriceHiller PriceHiller force-pushed the test/minimal_init_improvements branch 3 times, most recently from 9c28651 to cc99dff Compare September 26, 2023 13:38
@PriceHiller
Copy link
Contributor Author

Those "4 commits" are rebases over previous commits to address the previous comments while keeping the git history clean as a heads up.

@PriceHiller
Copy link
Contributor Author

*6 commits now :/

Missed the CI checkout for the deps. I haven't found anything else, unless you find something I think (see hope) this is good to go.

Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

Let's just clean up the workflow steps that are not necessary any more and I think it's good to go.

.github/workflows/tests.yml Outdated Show resolved Hide resolved
We now download these deps as part of the minimal init
Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

Everything looks good now, thanks!

@kristijanhusak kristijanhusak merged commit a659718 into nvim-orgmode:master Sep 26, 2023
4 checks passed
@PriceHiller PriceHiller deleted the test/minimal_init_improvements branch September 26, 2023 14:43
SlayerOfTheBad pushed a commit to SlayerOfTheBad/orgmode that referenced this pull request Aug 16, 2024
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.

2 participants