#1653026: [META] Use properly typed values in module configuration seems to have broken deployment. #2105771: block.block.bartik.breadcrumbs and block.block.seven.breadcrumbs always show as need importing is one bug, there are lots of others out there we don't know about yet.
Because the process of setting up a dev/live deployment scenario is extremely time-consuming and error-prone to do manually, it's not shocking to me that people don't do it often when testing CMI patches. It is, however, the entire point of that feature, so we should make sure that it's always working even with various refactorings we do.
So we need a test that does (at least) something roughly like this:
- Set up prod
- Set up dev based on prod
- Change site slogan on dev
- Export config from dev
- Import config on prod
- Verify that site slogan and only site slogan appears on the synchronize list
- Import the changes on prod
- Verify that prod's synchronize list is now empty.
- Verify site slogan changed on prod
- Also do the same with something fancier, like make a content type with a field it on dev
...Because those are what I tried to do in front of an audience of 70 people or so today at PNWDS and it failed. Hard. :)
chx says the following could work for writing the test (there's some trickiness because test methods aren't guaranteed to run in any particular order, and order matters here):
1) Add a sort($class_methods) to TestBase::run based on a property
2) Write a test method that exports
3) At the end of the test do $this->export = file_get_contents('exported_tarball'); (to hold the contents between testruns so they dont' get deleted)
4) write a test method which is alphabetically later which imports (testExport, testImport will actually do)
Each testX method automatically spins up a new Drupal site, so the dev/prod would easily be covered.
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff.txt | 3.17 KB | chx |
#12 | 2106171_12.patch | 4.42 KB | chx |
#11 | interdiff.txt | 1.91 KB | chx |
#11 | 2106171_10.patch | 3.91 KB | chx |
#8 | 2106171_7.patch | 3.37 KB | chx |
Comments
Comment #1
chx CreditAttribution: chx commentedThe test is here, the config import fails with
Attempt to save a configuration entity <em class="placeholder">user.register</em> with UUID <em class="placeholder">c1d211c4-7b4a-49e8-b900-0f69eab900f1</em> when this entity already exists with UUID <em class="placeholder">cd6e2b56-b951-4166-b435-53168378ea84</em>
(I have dumped the exception message from ConfigSync::submitForm)Comment #3
dawehnerI think this is caused by #1969800: Add UUIDs to default configuration
Comment #4
chx CreditAttribution: chx commentedThis passes. Includes the patch from #3 PLUS a few belonging to that issue PLUS a fix to user module overriding the imported action config entities. Also, we have a WTF:
import configuration
permission is effin' useless -- you can upload your file but then get a 403 because the sync page requiressynchronize configuration
. That's a followup to reconsider.Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedabsolutely love this patch! chx++
needs work for that debug.
i like the innovation of sorting the test functions, and reusing the config zip file. that nicely tests 'two completely separate installs'.
i'm not sure if we do this as a follow up or not, but 'two completely separate installs' is just one way to set up envs.
using a db dump and rsync is also a valid use case, so i wonder if we should have a test along those lines as well? i tested this quick hack of ConfigExportImportUITest and it seems to work:
this should be another test class, and possibly a follow up commit. will wait for feedback before posting a for-reals patch.
Comment #6
chx CreditAttribution: chx commentedI think that's a followup, sorry. This patch is quite badly needed. The real big followup will be setting the testing profile to standard....
Comment #7
webchickI committed #1969800: Add UUIDs to default configuration and #2108419: user module overwrites role actions breaking config sync, so this'll definitely need a re-roll now.
GO CHX GO! :D
Comment #8
chx CreditAttribution: chx commentedNow the other two is in so YOU SHALL PASS.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedRTBC if it comes back green.
Comment #10
webchickSorry, not quite. It still needs to pass the docs gate. (Not marking needs work for that though so that we can see what testbot says.)
Comment #11
chx CreditAttribution: chx commentedI can't believe we are wasting time on writing doxygen for test methods. Filed #2108785: [policy] Remove the requirement for doxygen for test methods.
Comment #12
chx CreditAttribution: chx commentedInterdiff is against #7! Because #7 has passed when this was posted.
Comment #13
webchickAwesome. :)
Committed and pushed to 8.x. Thanks!
Comment #14
webchickFollow-ups:
#2108809: Make config import/export tests pass with standard profile
#2108811: Refactor config import/export tests to allow for easy extension
#2108813: Add fancier config import/export test scenario
Comment #15
webchickUpdating title to reflect what was actually done here.
Comment #16
webchick.