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

First pass at Behat integration tests for Solr Power #95

Merged
merged 6 commits into from
Jul 20, 2016

Conversation

danielbachhuber
Copy link
Contributor

@danielbachhuber danielbachhuber commented Jul 18, 2016

See #79

@danielbachhuber
Copy link
Contributor Author

#84 is going to conflict with this PR's dependencies, so I haven't added them yet.

@danielbachhuber danielbachhuber added this to the 0.5 milestone Jul 19, 2016
@danielbachhuber
Copy link
Contributor Author

@stevector This is ready for a review. Note: I didn't commit the changes to the /vendor/ directory. Not sure what we want to do there.

override:
- ./bin/behat-test.sh
post:
- ./bin/behat-cleanup.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

@danielbachhuber Can you comment this out until tests are passing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the source of the failure:

image

What should we do about the vendor directory generally?

Copy link
Contributor

Choose a reason for hiding this comment

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

From our meeting yesterday with @ataylorme we agreed to that the ideal set up for the vendor dir would be:

  • No vendor directory in github.
  • Treat wordpress.org's SVN as a repository of build artifacts that would include the full vendor directory.
  • Some degree of automation sending build artifacts to SVN in response to tags on github.

However, right now our master branch here on github does have the vendor directory fully committed. I think for the scope of this PR we should not be changing anything related to the vendor directory unless we have to. The initial PR should simply automate (what I assume is) an already functional set of install instructions.

Having never installed this plugin myself, I'm not certain if this failure is because something in the vendor directory needs to change or something in the set up script needs to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Treat wordpress.org's SVN as a repository of build artifacts that would include the full vendor directory.

How do we accommodate the fact that some dependencies are only for testing (e.g. development), and don't make sense in the final build artifact?

Having never installed this plugin myself, I'm not certain if this failure is because something in the vendor directory needs to change or something in the set up script needs to change.

@ataylorme Mind taking a look at this failure and weighing in with your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

How do we accommodate the fact that some dependencies are only for testing (e.g. development), and don't make sense in the final build artifact?

We use Composer's distinction between require and require-dev. When building for SVN we would use composer install --no-dev

We have a mishmash of dependencies right now, which means `composer
install` isn't safe to run before the plugin has been pushed.
@danielbachhuber
Copy link
Contributor Author

@stevector @ataylorme Fixed this up. Could I get a #reviewmerge so I can move on, and we can fix the failed builds on the other PRs?

@ataylorme
Copy link
Contributor

@stevector @ataylorme Fixed this up. Could I get a #reviewmerge so I can move on, and we can fix the failed builds on the other PRs?

@danielbachhuber Thanks for this. The tests in tests/behat/solr-power.feature look good to me and I verify they passed on CircleCI.

Once we merge the facet widget we will want to expand tests to include indexing and searching for results but that's more of a would like to have.

The current checks that the plugin is loaded by visiting the options page and that Solr is available are good for now.

@ataylorme ataylorme merged commit 492b670 into master Jul 20, 2016
@jazzsequence jazzsequence deleted the 79-behat-integration-tests branch April 6, 2023 21:10
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.

3 participants