-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: add a more robust set of tests #85
Conversation
rebased, and try a fix as something was failing |
tests are green |
LGTM. I have done it in a while, but I want to make sure our tests can still pass locally. Could you please check that and document at the bottom of the README that command to run? |
It will run the default core
@weitzman I did run the tests manually several times but in testing this again I noticed some potential issues by not using the new I fixed this and now just running it locally works and uses the core default. Added to the readme, latest version can be read on https://github.com/hanoii/ddev-drupal-contrib/blob/tests-rehaul/README.md#add-on-tests EDIT: Added some small tweaks to the test |
Still doing some work |
@weitzman it's ready for review, I decided to use git submodule for My rationale for this is that it's a lot easier to install and to depend on things like bats-assert. Updated the README but the gist to run this locally is: git submodule update --init
./tests/bats/bin/bats tests Hope you like it, it was quite a bit of work getting it right! EDIT: commands simplification |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well and as intended.
I like the detailed test output.
I've had bad expereience with git submodules before so I tend to avoid them, however, it worked locally for me here. And more importantly didn't conflict with my local bats
brew setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any time somebody goes to the crazy effort to expand and improve tests I have to be happy!
I'm pretty sure we don't need specialty handling for d10/d11.
I wouldn't have put the bats extensions in git submodules because I've always hated them, but the README makes it sound easy to activate them.
Co-authored-by: tyler36 <[email protected]>
It's true that currently there's no difference, but... I did experience an issue: #82, while not with the functionality, the way it was suggested to install yarn, didn't work as is with d11. I figured it wasn't bad that we have both cores. And with that, I am also testing the ability to change cores and making sure that's being picked up. |
Most suggestion were addressed but one which I followed up. Thanks |
My suggestion of |
but the code is not already layed out in the context of that test, as we are testing adding the core later on as part of |
I was going to add some thoughts on the git submodules discussion, only because I feel it's interesting... but added it to ddev/ddev-addon-template#62 instead |
If the code isn't there yet, then using |
Adding something more useful in terms of test coverage, for now this adds:
Different drupal version and a smaller set of tests are run by GHA strategy matrix
What I like to add in this initial approach is that eslint and stylelint actually does something, but that can be added later.
The tests will fail until #81 and #80 are accepted.They are in, tests should now be green