biocore / american-gut-web

The website for the American Gut Project participant portal
BSD 3-Clause "New" or "Revised" License
5 stars 24 forks source link

To be able to test the metadata pulldown for additional surveys like … #630

Closed sjanssen2 closed 7 years ago

sjanssen2 commented 7 years ago

…"Fermeted Food" and "Surfer" we need to have some test data. With this patch, manually generated test data is added, such that there is something to pull down.

Barcodes to pulldown are: 000033796 000033797 000033798 000033799 000033800 000033801 000033802

I will add this list of barcodes to labadmin test dir.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 63.758% when pulling 8a17125a943fc73ee710b596bd98eea8f6bc4c50 on sjanssen2:addedSurveyTestData into d4b9f0606ea8f54516f7c8cdaf6fd191fea442ed on biocore:master.

sjanssen2 commented 7 years ago

Could someone please review this PR now @wasade @EmbrietteH @mortonjt It blocks labadmin dev, due to failing tests against the new data.

EmbrietteH commented 7 years ago

Quick clarification question-how were the test data generated? These barcodes belong to real participants, will there be any issues of conflict (i.e. is there ever a chance that we accidentally associate the test data as real data with these participants)? Thanks!

sjanssen2 commented 7 years ago

About the history: I set up labadmin and agweb on my machine. Via labadmin, I added some new barcodes to existing kits. Then, I used the agweb website to manually log in with the kit credentials added human and animal sources, filled out the surveys and associated them to the new barcodes. Using a diff between DB dumps before and after those operations gave me the patch 0040.sql.

Until now, I don't understand how the productive system is set up. Does it also apply the patch files? If yes, there is the risk of confusing these mock surveys as real data. Ideally, someone with the right permissions greps real data for the food, surver, animal and personal microbiome surveys and creates an alternative patch?!

I am not sure if we ever re-do the process of taking real data, overwrite most of the fields with "REMOVED" and fed that back to our db that is populated by setting up labadmin. That might cause some other conflicts.

josenavas commented 7 years ago

I don't think this is the correct way of adding test data to the database. Note that the the patches are also applied to the live system. i.e. if this PR is merged, and we deploy in the live system, I see that the live database contains patch 39 and the code contains patch 40. I will apply this patch in the live system, which means that this data gets inserted in the live system.

Patches are applied in both systems (test and live) so patches are mainly used to modify DB structures/data that need to be in the live system.

If not having this data is a blocker for testing, I think we need to go back and create another test database with the "REMOVED" information so we can perform testing. @wasade what do you think? This would mean to replace ag_test_patch22.sql.gz with ag_test_patch39.sql.gz and update this line of code.

mortonjt commented 7 years ago

@sjanssen2 @josenavas what about having a testing patch? I'd think that having some testing data would be important in creating test cases.

josenavas commented 7 years ago

if you can apply that patch in the setUp of the test that would work.

josenavas commented 7 years ago

but it is technically not a patch - it is just a bunch of data that you're inserting in the DB

sjanssen2 commented 7 years ago

I just realize how many data are missing for good tests. Thus, I would really appreciate a new round of deidentifying the productive DB with all the filled out surveys and feed that back into the DB that is set up when installing american-gut-web from biocore!

sjanssen2 commented 7 years ago

Let's close this PR because it is the wrong method. I am now dealing with two SQL files to inject and remove necessary data via the unittest setup() and teardown() methods. However, there is urgent need for suitable, de-identified test data in the default database. So please @josenavas and/or @wasade update patch22!

wasade commented 7 years ago

@sjanssen2, we should not update existing patches but add a new one. I'll track down the method for generating test database and fwd