Heeeeeeeeeeeeere's some tests. It requires two things... first, it requires all of those patches below because this targets the mapping form, and the mapping form's AJAX validation problem was causing ALL SORTS of problems when it came to trying to build a test against it. Secondly, it requires that you have authenticated with Salesforce on the site that is running the simple tests. I tried to replicate the actual OAuth procedure but was having problems during the redirect sequences. Eventually I decided to "steal" the data from the primary site and import it into the temporary test site. The SalesforceTestCase class has the salesforceConnect() method, so as long as all the tests are a child of that class, then everyone should have access to connect to Salesforce.
This patch requires the following other patches to be applied first:
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | salesforce_mapping-tests-1951744.patch | 2.13 KB | ceardach |
| #12 | salesforce_mapping-tests-1951744-12.patch | 60.03 KB | kostajh |
| #11 | salesforce_mapping-tests-1951744-11.patch | 60.01 KB | kostajh |
| #6 | salesforce_mapping-tests-1951744.patch | 60.94 KB | ceardach |
| #2 | salesforce_mapping-tests-1951744.patch | 58.6 KB | ceardach |
Comments
Comment #1
kostajh commentedComment #2
ceardach commentedSome of the tests needed to be updated with the latest in HEAD.
Comment #3
levelos commentedComment #4
bleedev commented@ceardarch Thank you for writing these tests, however there have been several updates recently and the patch no longer can be applied correctly. Currently the tests are failing, although I am not sure that is because of the tests or the recent updates. Could you please re-roll against the latest code?
It also might be worth discussing the best way to get credentials for the tests. The current method has the potential to allow updates to production data.
Comment #5
levelos commentedCorrecting status.
Comment #6
ceardach commentedTests are rerolled, and also updated to catch the bug that got introduced but was unable to catch. Apparently, for the life of me, I just could not figure out how to assert the value of a radio field. So, finally, I just had to make my own assert method. Sheesh.
It is possible my tests rely on #1951728: Allow those using soap to use both "externalId" and "idLookup". I think I recently found an externalId so I could possibly switch over the tests to use a field on that instead of idLookup.
Yes, I spent a lot of time trying to figure out the authentication. I tried to get simpletest to go through the whole authentication process itself, but it would fail on the redirects. Eventually I settled on stealing the data from the primary site's database. I have concluded that its OK because I believe that the vast majority of sites running simpletests are *not* production sites... right? I would think the performance degradation would prevent even the average Joe from wanting to run them in their production environments.
And lastly, I have been trying to regularly monitor the tests to ensure that they are non destructive or intrusive on the Salesforce environment. I've been keeping my environment clean with test records to be kept "clean" and maintained, and make sure that all test records get cleaned up after themselves, so that even if it is run against an environment that should be preserved, there would be minimal risk of contamination.
But, I am certainly open to suggestions.
Comment #7
ceardach commentedComment #8
kostajh commentedFinally had a chance to test this out.
It's possible that #1951728: Allow those using soap to use both "externalId" and "idLookup" is a dependency as running the tests results in
235 passes, 299 fails, 11 exceptions, and 65 debug messagesfor Mapping UI. Mapping Entities tests were fine though.I'm also seeing this warning
Strict warning: Declaration of SalesforceMappingMapTestCase::setUp() should be compatible with that of SalesforceMappingTestCase::setUp() in _registry_check_code()Comment #9
levelos commentedComment #10
kostajh commentedHere's the patch updated for external ID.
Looks like #1978090: Validate that mapped properties have appropriate getter and setters is causing some issues with the tests (e.g. "Created" field doesn't have a setter method).
Comment #11
kostajh commentedComment #12
kostajh commentedHere's an updated patch so that all tests pass. I'd like to get this in as it's useful for testing the mapping form and can help us catch issues before tagging releases in the future.
There's a lot more that could be done but it's a good framework that we can build upon. Thanks @ceardach for all your work on this!
Comment #13
kostajh commentedThis is committed in 4452169
Comment #14
ceardach commentedhmmm were these passing for you when you committed? The tests aren't passing for me. I'm wondering if its an environment difference or something.
Comment #15
ceardach commentedI attached a patch that fixed one problem. For some strange reason applying the latest patch for #1951728: Allow those using soap to use both "externalId" and "idLookup" fixes the rest of the problems. I really have no idea why.
Comment #16
ceardach commentedComment #17
kostajh commentedGiven the issues with #1951728: Allow those using soap to use both "externalId" and "idLookup", I think it would be best to remove the tests for Key and create a follow up issue for that. What do you think?
Comment #18
levelos commentedI would like to revisit the overall approach to testing taken so far. I don't think that pulling SF credentials from an underlaying Drupal installation is a safe or reliable approach and not consistent with other 3rd party integration tests for Drupal modules. E.g., Mollom creates its own test "server" which is used to test the Mollom Drupal module integration. It basically spoofs API functionality. We're now doing the same thing for our MailChimp suite. Something similar should probably be done for SF. Regardless, I'm not comfortable leaving these tests.
I'll leave this open for feedback for bit before rolling back the current tests
Comment #19
Bastlynn commentedPerfection is the enemy of progress. Imperfect tests are better than no tests.
Comment #20
levelos commented@Bastlynn, well put, and I'm open to discussion, but simply taking the credentials from an underlying production database can lead to problems for many use cases, prevents the tests from running in any kind of automated testing infrastructure, including d.o., and cannot be used as a platform for further and more robust testing. All we're testing now is the mapping form, which is great, but that's not where most of the testing needs to take place.
In short, what we have now is not a starting point for adding more test coverage, is not a best practice for testing 3rd party integration, and can lead to unattended data access and potential corruption in production environments. These critical points are very different from saying make the tests comprehensive or don't include them at all, which is more what your statement speaks to.
Comment #21
kostajh commentedAs it stands, the tests are useful for catching breaking changes in patches that affect the mapping interface as well as mapping entities.
For that reason, my opinion is to leave the tests in until someone can contribute a Salesforce Test Server module along the lines of the Mollom and Mailchimp test suites.
Either way, I think much of the code is reusable so it's not wasted effort.
Comment #22
ceardach commentedThe connection method currently being used is tentative, yet the other possible methods are also difficult with Salesforce. This particular 3rd party integration system is going to need a bit more thinking and perhaps a few concessions. I for one am out of ideas, that is why I chose the method I did. As Bastlynn said, imperfect testing is better than no testing.
At this moment, the risks of this connection method is so minimal it could be considered negligible. The current test is read-only, so there is no risk of data manipulation or corruption if someone mistakenly ran this against a production instance. In that case, it can remain as-is until a consensus for a connection method is reached.
Comment #25
aaronbauman7.x is no longer supported