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

Ensure Chado Prepare works in automated testing #297

Merged
merged 42 commits into from
Nov 15, 2022

Conversation

laceysanderson
Copy link
Member

@laceysanderson laceysanderson commented Nov 3, 2022

This PR adds

  • basic tests to ensure that the prepare step runs successfully in the automated testing environment
  • fixes to all functionality called during prepare to address bugs found during this process
  • adds a prepareTestChado() method to ChadoBrowserTestBase to efficiently prepare in automated tests
  • Ensure that the prepare step is producing the vocab terms, etc that it should.
  • Use the same testing in the previous point to ensure the efficient testing method is also producing the vocab terms, etc that it should.

Testing

Manual

  • Check that you can run the prepare task through the UI without errors.

Automated testing

  • checks that the prepare task completes without error in the automated testing environment
  • Check that $prepared_chado = $this->getTestSchema(ChadoTestBrowserBase::PREPARE_TEST_CHADO); provides a new test chado instance without error.
  • checks that both the above two chado instances have the following
    • the custom tables + materialized views added by the prepare
    • the cv, db, cvterm, dbxref tables all have the expected number of records
    • a specific subset of cvterms are checked directly
    • the two mviews populated by prepare have been populated with the expected number of records.

Documentation

There has been a documentation issue opened tripal/tripal_doc#13

Also, to get a chado instance prepared for testing use the following command in any test which extends ChadoBrowserTestBase:

$prepared_chado = $this->getTestSchema(ChadoTestBrowserBase::PREPARE_TEST_CHADO);

Warning

Previous versions of this PR had a prepareTestChado() method WHICH HAS BEEN REMOVED. Please use the new approach instead.

risharde and others added 30 commits October 24, 2022 16:07
Bringing both prepare testing branches together
@laceysanderson
Copy link
Member Author

I believe the tests should now pass and that this PR successfully adds an efficient way to get a prepared chado for testing and fixes all the bugs which stopped the official prepare task from running in the testing environment.

To create a prepared chado test instance you do the following:

$prepared_chado = $this->getTestSchema(ChadoTestBrowserBase::PREPARE_TEST_CHADO);

Note: this is different then we originally discussed as it became clear we did not want to add records to an existing chado test db due to too many unexpected issues. This approach is more reliable and just as fast. The only downside is that ChadoTestBrowserBase creates an empty chado that we do not use when this approach is taken which is a bit more overhead then needed.

We also need to add documentation for automated testing in the Tripal/Chado environment. I've added an issue with some details here: tripal/tripal_doc#13

@codeclimate
Copy link

codeclimate bot commented Nov 7, 2022

Code Climate has analyzed commit 0e6211a and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 73.3% (50% is the threshold).

This pull request will bring the total coverage in the repository to 33.2% (12.8% change).

View more on Code Climate.

@laceysanderson
Copy link
Member Author

Confirmed that since this is well tested, has been up for more then 3 days it can be merged 🎉 This will be used a lot in future testing so if there are any bugs -they will be found :-)

@laceysanderson laceysanderson merged commit c4ce113 into 9.x-4.x Nov 15, 2022
@laceysanderson laceysanderson deleted the tv4g1-issue291-testChadoPrepare branch November 15, 2022 22: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