Integration tests #38

lschaefer-sugarcrm · 2018-03-28T14:21:22Z

@glevine-sugarcrm Are you able to review the unit tests? I think I mostly stuck with what you suggested.

@mmarum-sugarcrm This PR is finally ready for review! If you're able to try out running the tests locally as well as in Jenkins, it might be really helpful. I think I've documented all of the little pieces you need to install and configure, but it's possible I missed something along the way. I've been opening Issues related to this work that can be finished separate from this PR.

mmarum-sugarcrm · 2018-03-28T15:30:09Z

README.md

+- DEV_COMMUNITY_USERNAME: The username for an account that has access to the 
+[SugarCRM Developer Builds Space](https://community.sugarcrm.com/community/developer/developer-builds)
+- DEV_COMMUNITY_PASSWORD: The password associated with the above account
+- GIT_HUB_USERNAME: The username for a GitHub account that has access to https://github.com/sugarcrm/unit-tests


nitpick. I would not break up GitHub name with underscore.

I actually waffled over this. I debated if GITHUB would make you read the th together like the th in thesaurus. Obviously, I overthought this. Updating.

mmarum-sugarcrm · 2018-03-28T15:36:58Z

scripts/config_override.php

+$sugar_config['developerMode'] = false;
+$sugar_config['dump_slow_queries'] = false;
+$sugar_config['slow_query_time_msec'] = '1000';
+$sugar_config['perfProfile']['TeamSecurity']['default']['teamset_prefetch'] = true;


Do not include these TeamSecurity settings. They need to be manually tuned for each Sugar instance based upon data within it.

mmarum-sugarcrm · 2018-03-28T15:37:07Z

scripts/config_override.php

+$sugar_config['perfProfile']['TeamSecurity']['default']['teamset_prefetch'] = true;
+$sugar_config['perfProfile']['TeamSecurity']['default']['teamset_prefetch_max'] = 500;
+$sugar_config['perfProfile']['TeamSecurity']['default']['where_condition'] = true;
+$sugar_config['import_max_records_total_limit'] = '2000';


mmarum-sugarcrm · 2018-03-28T15:37:30Z

scripts/config_override.php

+$sugar_config['perfProfile']['TeamSecurity']['default']['teamset_prefetch_max'] = 500;
+$sugar_config['perfProfile']['TeamSecurity']['default']['where_condition'] = true;
+$sugar_config['import_max_records_total_limit'] = '2000';
+$sugar_config['verify_client_ip'] = false;


Remove unless it's absolutely necessary. This is a security feature.

mmarum-sugarcrm · 2018-03-28T15:39:01Z

scripts/config_si.php

+    'setup_fts_host' => 'sugar-elasticsearch',
+    'setup_fts_port' => '9200',
+    'setup_fts_type' => 'Elastic',
+    'setup_license_key_expire_date' => '2017-03-18',


remove setup_license_* stuff.

mmarum-sugarcrm · 2018-03-28T15:39:07Z

scripts/config_si.php

+    'setup_num_lic_oc' => 0,
+    'setup_site_admin_password' => 'sugarcrm',
+    'setup_site_admin_user_name' => 'admin',
+    'setup_site_sugarbeet_automatic_checks' => false,


Remove this.

mmarum-sugarcrm · 2018-03-28T15:39:29Z

scripts/config_si.php

+    'setup_db_admin_user_name' => 'root',
+    'setup_db_admin_password' => 'root',
+    'setup_db_create_database' => 1,
+    'setup_db_create_sugarsales_user' => 0,


remove this.

mmarum-sugarcrm · 2018-03-28T15:40:39Z

scripts/config_si.php

@@ -0,0 +1,36 @@
+<?php
+$sugar_config_si = array (


Can we use the config_si example from here? http://support.sugarcrm.com/Resources/Environments/Development_Environments/Vagrant_Development_Environment/

The only things that should NEED to be different would be usernames, passwords, hostnames, etc.

lschaefer-sugarcrm · 2018-03-29T16:32:53Z

@mmarum-sugarcrm I've made the updates. Can you take another look? The build is passing in both Jenkins and Travis. I'm still debugging why the Prof M MLP isn't installing cleanly locally on my machine.

Update config_override Update GIT_HUB to GITHUB Update config_si Fix headers error Remove sudo from calls to get install to work on local machine

glevine · 2018-04-02T15:27:24Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+
+use Sugarcrm\Sugarcrm\custom\gradebook_fake\RecordManager;
+
+class StudentGradebookJob implements RunnableSchedulerJob {


Our internal styleguide says to put the braces for class declarations on their own line. It's not super important, but if you want to adhere to our standards, then you might want to move the { to the next line.

Thanks for the detailed review! I've made the style changes you made and opened an issue to track adding a style checker to our build process.

glevine · 2018-04-02T15:28:56Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+     */
+    public function run($data)
+    {
+        if (!empty($data))


PSR-2 says that the brace should be on the same line as the if. "Opening braces for control structures MUST go on the same line, and closing braces MUST go on the next line after the body."

glevine · 2018-04-02T15:29:44Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+        {
+            $bean = $this->getContactBean($data);
+
+            try{


PSR-2 says "Control structure keywords MUST have one space after them; method and function calls MUST NOT." So try { and if ($success) { and } else {.

glevine · 2018-04-02T15:31:22Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+
+    /**
+     * This function defines the job that adds a new student to the GradebookFake app
+     * @param $job Information about the job. $job->data should be the id for the new Student (contact) record


Your parameter is actually called $data. You're also missing a type. It looks like it's an ID, so string would really be the only supported type.

glevine · 2018-04-02T15:32:09Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+        return false;
+    }
+
+    protected function getContactBean($id){


Braces for functions go on their own line.

I would also suggest having doc blocks for all methods since people will read the code and ask questions for every method, no matter how minor and seemingly obvious.

glevine · 2018-04-02T16:21:19Z

package/src/custom/tests/School/unit-php/modules/Contacts/Student_GradebookTest.php

+    public function testAddStudentToGradebook_DoesNotCreateTheJob_NotAfterSaveEvent()
+    {
+        $this->sg->addStudentToGradebook($this->student, 'after_relationship_add', ['isUpdate' => false]);
+        $this->queue->expects($this->never())->method('submitJob');


You need to set your expectation before you call addStudentToGradebook.

glevine · 2018-04-02T16:21:33Z

package/src/custom/tests/School/unit-php/modules/Contacts/Student_GradebookTest.php

+    public function testAddStudentToGradebook_DoesNotCreateTheJob_IsUpdateEvent()
+    {
+        $this->sg->addStudentToGradebook($this->student, 'after_save', ['isUpdate' => true]);
+        $this->queue->expects($this->never())->method('submitJob');


You need to set your expectation before you call addStudentToGradebook.

glevine · 2018-04-02T16:30:51Z

package/src/custom/tests/School/unit-php/phpunit.xml.dist

+    <testsuites>
+        <testsuite name="all">
+            <directory>./</directory>
+            <directory>modules/</directory>


Doesn't ./ includes modules/? It should be recursive. If you got rid of this second line, then would it still run everything?

I removed it and the tests still ran. I copied it out of the regular Sugar phpunit.xml.dist, so it's odd they have it there as well.

I agree. I don't quite know why it was added in the first place. If you wanted to create a test suite that runs only tests that validate the logic hook handlers, then you might have a "logic hook" suite that specifies only the files and/or directories where those tests exist. But I don't think you need to worry about that yet. Running everything is fast.

glevine · 2018-04-02T16:44:41Z

package/src/custom/tests/School/unit-php/phpunit.xml.dist

+    </testsuites>
+    <filter>
+        <whitelist processUncoveredFilesFromWhitelist="false">
+            <directory suffix=".php">../../data/</directory>


None of these directories are from your package. You only care about code coverage for the code you write, so this should probably be a list of directories or files that are installed by your package. If you tell me the paths of all of the files that are installed, then I can help you figure out what entries need to be here.

https://phpunit.de/manual/current/en/appendixes.configuration.html#appendixes.configuration.whitelisting-files

Great catch! I've set it (as a first start) to just get code coverage of the custom directory. It's tricky as the custom directory gets some files when Sugar is installed that are not related to our Professor M MLP. I'm concerned that if we get more specific with the file listings, we'll forget to update this as we add more customizations.

Here is a list of all of the files I sync from my installed Sugar directory to my git repo. I'm thinking we don't want a list like this in phpunit.xml.dist. What do you think?
- /Applications/MAMP/htdocs/profm711unittestsb/tests/unit-php/custom//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/gradebook_fake//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/tests//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Schedulers//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Contacts/Ext/LogicHooks/profm.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Accounts/Accounts_Save.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Accounts/logic_hooks.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Contacts/Students_Gradebook.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules//professor.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules//Professor.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Accounts/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Accounts/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-accounts-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Calls/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-calls-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Calls/Ext/Vardefs/rli_link_workflow.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Campaigns/Ext/Layoutdefs/_overrideCampaign_subpanel_opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Contacts/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-contacts-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Contacts/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Documents/Ext/Layoutdefs/_overrideDocument_subpanel_opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Documents/Ext/Vardefs/rli_link_workflow.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Leads/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Meetings/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Notes/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-notes-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Notes/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Opportunities/Ext/Dependencies/opp_disable_dep.ext.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Opportunities/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Products/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-products-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Products/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Project/Ext/Layoutdefs/_overrideProject_subpanel_opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Project/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Quotes/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-quotes-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Quotes/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/RevenueLineItems/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-revenuelineitems-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/RevenueLineItems/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Tasks/Ext/clients/base/layouts/subpanels/_overridesubpanel-for-tasks-opportunities.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/Extension/modules/Tasks/Ext/Vardefs//*
- /Applications/MAMP/htdocs/profm711unittestsb/icons//Professor.*
- /Applications/MAMP/htdocs/profm711unittestsb/language/application/en_us.lang.php
- /Applications/MAMP/htdocs/profm711unittestsb/metadata//professor.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Accounts/clients/base/views/list/list.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Accounts/clients/base/views/record/record.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Contacts/clients/base/views/list/list.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Contacts/clients/base/views/record/record.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Leads/clients/base/filters/default/default.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Leads/clients/base/layouts/convert-main/convert-main.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Leads/clients/base/views/list/list.php
- /Applications/MAMP/htdocs/profm711unittestsb/custom/modules/Leads/clients/base/views/record/record.php
- /Applications/MAMP/htdocs/profm711unittestsb/modules/PR_Professors//*
- /Applications/MAMP/htdocs/profm711unittestsb/modules/RevenueLineItems/metadata/studio.php

Is there an easy way to view a code coverage report? I'm curious what it shows as being covered.

glevine · 2018-04-02T16:49:39Z

scripts/RunProfMPHPUnitTests.sh

@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+
+# This script runs the Sugar PHPUnit tests. It assumes the Sugar Docker stack has already been started and that


This runs "your" tests, not the Sugar tests, right?

Good catch!

glevine · 2018-04-02T17:25:35Z

@lschaefer-sugarcrm - You now have all of your custom code tested, assuming it all exists in the files...

src/custom/modules/Contacts/Students_Gradebook.php
src/custom/gradebook_fake/RecordManager.php
src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

... as the rest looks like metadata changes. You likely have 100% code coverage and have tested all of your edge cases. So now you could write a single integration test that tests a complete happy path for the entire flow from saving a Contact to doing whatever RecordManager is going to do (If it never really does anything, then verifying that the job was completed successfully is probably your best bet for validating the path.). With that you'll have a fairly comprehensive testing story that includes tests at various layers within the testing pyramid (e.g., most of your tests are true unit tests that help you isolate issues based on very specific conditions and a very small number of integration tests -- in this case one -- that make sure your code will produce the correct result when integrated into a system that behaves according to the assumptions upon which your code was designed).

You'll run into some problems, however. The biggest one is that we don't currently have a way of testing asynchronous flows, like those that execute using the queue. You would end up having to write a test that executes the queue and then checks the status of your job, over and over until completion. That would not be ideal as it means your test would have unnecessary complexity and the runtime could be excessive because you don't know how long it will take to run.

I think you should file a story on the AT board for creating an API that would enable executing the job queue to run a specific job or a specific set of jobs, whether it be by name or ID or something else. Then an integration test could be written that does some work, creates a job, executes only the job or jobs that matter for that test, and asserts the results.

lschaefer-sugarcrm · 2018-04-03T16:11:27Z

@glevine Thanks for the info on writing an integration test! I filed an internal ticket as you suggested. I also opened Issue #43 to track adding the integration test.

I think I've addressed your other suggestions.

glevine · 2018-04-05T20:02:15Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+            try {
+                //Call the external GradebookFake app to create a new record in it
+                $rm = $this->getRecordManager();
+                $success = $rm->createStudentRecord($bean->email1, $bean->first_name, $bean->last_name);


email1 shouldn't be used anymore. SugarEmailAddress::getPrimaryAddress() can be used to get the string of the primary email address. You don't have to change this now, but you'll want to create an issue to remind you to do it later.

Since I was messing with the file anyway, I went ahead and fixed this.

glevine · 2018-04-05T20:03:02Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+                //Call the external GradebookFake app to create a new record in it
+                $rm = $this->getRecordManager();
+                $success = $rm->createStudentRecord($bean->email1, $bean->first_name, $bean->last_name);
+                if ($success){


You should have a space before the {.

glevine · 2018-04-05T20:03:16Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+                if ($success){
+                    $this->job->succeedJob();
+                    return true;
+                } else{


You should have a space before the {.

glevine · 2018-04-05T20:06:49Z

package/src/custom/Extension/modules/Schedulers/Ext/ScheduledTasks/StudentGradebookJob.php

+     */
+    protected function getContactBean($id)
+    {
+        return BeanFactory::getBean('Contacts', $id);


BeanFactory::getBean() always returns a bean. If it can't find the right one by the $id then it just returns an empty bean of that type (Contacts). BeanFactory::retrieveBean() will return null if the record can't be found, which is what I think you want.

glevine · 2018-04-05T20:07:17Z

package/src/custom/modules/Contacts/Students_Gradebook.php

-            //push into the queue to run
-            $jq = new SugarJobQueue();
-            $jobid = $jq->submitJob($job);
+        if($arguments['isUpdate']){


You should have a space after if and another space before {.

glevine · 2018-04-05T20:07:38Z

package/src/custom/modules/Contacts/Students_Gradebook.php

+        return $jq->submitJob($job);
+    }
+
+    protected function getSchedulersJob()


Missing doc block.

glevine · 2018-04-05T20:07:46Z

package/src/custom/modules/Contacts/Students_Gradebook.php

+        return new SchedulersJob();
+    }
+
+    protected function getSugarJobQueue()


Missing doc block.

glevine · 2018-04-05T21:05:19Z

package/src/custom/tests/School/unit-php/phpunit.xml.dist

+    </testsuites>
+    <filter>
+        <whitelist processUncoveredFilesFromWhitelist="false">
+            <directory suffix=".php">../../</directory>


At this point, I think these are the only files you want to generate code coverage for. You could continue to add to custom/gradebook_fake. You could also make changes to custom/Extension/modules/Schedulers/Ext/ScheduledTasks/AddStudentToGradebookJob.php and/or custom/modules/Contacts/Students_Gradebook.php. So I think you should have this as your whitelist:

<directory suffix=".php">../../custom/gradebook_fake</directory> <file>../../custom/Extension/modules/Schedulers/Ext/ScheduledTasks/AddStudentToGradebookJob.php</file> <file>../../custom/modules/Contacts/Students_Gradebook.php</file>

That would only generate code coverage for your code. I don't think you can avoid having to maintain this list. It's just going to have to be a part of your DoD. If you add a new class to custom/modules/Accounts, then you'll need to add that on its own. But if you add another class to custom/gradebook_fake, then you won't need to add it because it'll already be captured by the <directory> directive.

Obviously, this is untested, so the only way to know if it works is to run it. You can either run it selectively from the command line or always run it.

To run it on the command line, you would run your tests with /path/to/phpunit --coverage-html /path/to/output/file. You just have to decide what type of coverage report you want to run (and the location where you want to write the file). The choices are:

--coverage-html /tmp/report --coverage-clover /tmp/coverage.xml --coverage-php /tmp/coverage.serialized --coverage-text > /tmp/logfile.txt --log-junit /tmp/logfile.xml --testdox-html /tmp/testdox.html --testdox-text /tmp/testdox.txt

To run it always, it's just another addition to your config file. Again, it's just a matter of choosing the type and the path to the file for output.

<logging> <log type="coverage-html" target="/tmp/report"/> </logging>

Thanks for the info on how to run the code coverage! I was able to get it working. I tweaked the settings and ended up with the following:

<whitelist addUncoveredFilesFromWhitelist="false" processUncoveredFilesFromWhitelist="false"> <directory suffix=".php">../../..</directory> <exclude> <directory suffix=".php">../..</directory> </exclude> </whitelist>

This sets the coverage to run on the custom directory and excludes the custom/tests directory. The downside to this is that if someone fails to test a file at all, it won't show up in the code coverage report. However, I feel more confident that we'll remember to check if a PR has appropriate testing than if the PR updates the phpunit.xml.dist. We could set this file and forget about it. What do you think?

mmarum-sugarcrm

+ bash -ex RunPackUnitTestsAndBuildProfMPackage.sh /Users/mmarum/jenkins/workspace/ProfM/
+ LOCALWORKSPACEPATH=/Users/mmarum/jenkins/workspace/ProfM/
+ docker rm my-yarn --force
my-yarn
+ docker rm my-composer --force
Error: No such container: my-composer
+ true
+ docker pull sugarcrmdev/school:yarn
yarn: Pulling from sugarcrmdev/school
Digest: sha256:de148b2af3883a5f24932bee10612b0d99b748ac81866efcff5a44465e3fdeae
Status: Image is up to date for sugarcrmdev/school:yarn
+ docker run -t -d -v /Users/mmarum/jenkins/workspace/ProfM/:/workspace --name my-yarn sugarcrmdev/school:yarn
5f51e60763bfac6912562aa0d6c6edd198b13c24469d996cee54ee45be446440
+ docker exec my-yarn yarn install
OCI runtime exec failed: exec failed: container_linux.go:348: starting container process caused "process_linux.go:86: executing setns process caused \"exit status 21\"": unknown
Build step 'Execute shell' marked build as failure
Finished: FAILURE

I see a couple errors
Error: No such container: my-composer
and
+ docker exec my-yarn yarn install OCI runtime exec failed: exec failed: container_linux.go:348: starting container process caused "process_linux.go:86: executing setns process caused \"exit status 21\"": unknown

I believe they are probably related. I'm not sure what the my-composer container is.

mmarum-sugarcrm · 2018-04-05T20:46:56Z

scripts/SetupEnvAndRunPHPUnitTests.sh

+if [[ -z "$1" ]] || [[ -z "$2" ]] || [[ -z "$3" ]] || [[ -z "$4" ]] || [[ -z "$5" ]] || [[ -z "$6" ]] || [[ -z "$7" ]]
+then
+    echo "Not all required command line arguments were set. Please run the script again with the required arguments:
+        1: Email address associated with your SugarCRM Developer Builds Community account


Instead of referring to Dev Builds space, I think we should just call this your Sugar Community username. Ultimately, access to dev builds space is tied to a download permission set on the account.
https://community.sugarcrm.com/docs/DOC-4842-on-boarding-new-sugar-developers

Agreed. I ended up fixing this in https://github.com/sugarcrm/school/pull/44/files (not yet merged).

mmarum-sugarcrm · 2018-04-05T21:27:11Z

scripts/RunPackUnitTestsAndBuildProfMPackage.sh

@@ -1,9 +1,12 @@
+# This script is designed specifically for Jenkins.  It runs the Jasmine and PHPUnit tests associated with the Pack
+# script.  It also builds the Professor M module loadable package.
+
 # If you are running Jenkins in a Docker container and mounting /var/jenkins_home to a directory on your machine's
 # local filesystem, you may need to update LOCALWORKSPACEPATH to reflect the path to the Jenkins workspace on your local


What's a Jenkins workspace? Can it be any directory?

No, Jenkins creates the directory automatically for you. Is this more clear?

If you are running Jenkins in a Docker container and mounting /var/jenkins_home to a directory on your machine's local filesystem (for example: /Users/lschaefer/jenkins), you may need to update LOCALWORKSPACEPATH to reflect the path on your local filesystem to the Jenkins workspace for your ProfessorM job (for example: "/Users/lschaefer/jenkins/workspace/ProfessorM"). The easiest way to update LOCALWORKSPACEPATH is to pass in the value as an argument when you call this script.

mmarum-sugarcrm · 2018-04-05T21:36:16Z

scripts/SetupEnvAndRunPHPUnitTests.sh

+        2: Password associated with the above account
+        3: Sugar version (Example: 7.11)
+        4: Sugar edition (Options: Ult, Ent, Pro)
+        5: GitHub username that has access to sugarcrm/unit-tests


This is a problem for me because I use 2FA. Github supports using SSH keys to authenticate. It would be good (and probably a better security practice) to have the option of using SSH keys.

Good point! I opened #51 to track this

mmarum-sugarcrm · 2018-04-05T21:42:32Z

README.md

+Jenkins).
+   1. Kind: **Secret text**
+   1. Scope: **Global** 
+   1. Secret: The path to where Sugar Docker will be stored on your host machine.  For example, if your Jenkins 


I don't really understand this piece. It's a bit tricky for me. You have to enter this path here and also in the Jenkins build script further below in a couple different paths. Is there a way to just provide a root path and then you figure out all the necessary relative paths?

I opened #52 to track this

mmarum-sugarcrm · 2018-04-05T21:46:07Z

.travis.yml

+    services:
+    - docker
+    env:
+    - SUGAR_VERSION=7.11 SUGAR_EDITION=Ent


Running tests on ent, pro, and ult seems overkill? The Professor M code doesn't come in different flavors. I assume you're doing this to run the core unit tests?

Notes from our chat: We're going to continue running on ent, pro, and ult as this tests the install into each version. I'm going to remove the running of the Sugar provided unit tests.

lschaefer-sugarcrm · 2018-04-06T14:23:27Z

@glevine The latest commit addresses your review. Changes based on Matt's review coming soon...

lschaefer-sugarcrm · 2018-04-06T14:31:22Z

@mmarum-sugarcrm Going to start going through your comments...

Error: No such container: my-composer
This one is somewhat expected. This command is removing a container if it exists from a previous build. If the container doesn't exist (because you haven't run the build before or the previous build cleanly removed everything), you'll get this error. I'll add some notes about it.

docker exec my-yarn yarn install OCI runtime exec failed: exec failed: container_linux.go:348: starting container process caused "process_linux.go:86: executing setns process caused \"exit status 21\"": unknown

This one is more of a mystery. Googling didn't bring up much. Can you update to the latest Docker and try again?

glevine · 2018-04-06T15:00:43Z

package/src/custom/tests/School/unit-php/phpunit.xml.dist

+    </testsuites>
+    <filter>
+        <whitelist addUncoveredFilesFromWhitelist="false" processUncoveredFilesFromWhitelist="false">
+            <directory suffix=".php">../../..</directory>


It's a little too magical for me. But it should work fine. It can always change later if something comes up that requires it.

Update README to reflect Sugar PHPUnit tests not run as part of build

lschaefer-sugarcrm · 2018-04-09T14:56:07Z

Based on conversation I had with @mmarum-sugarcrm on Friday, I'm going to merge this PR. I addressed the big thing he wanted me to fix (removing the Sugar unit tests from the builds) and opened new issues to track the remaining things.

lschaefer-sugarcrm added 2 commits February 14, 2018 10:54

Create script to download Sugar from Dev Builds community

af25f27

Add PHPUnit tests for Prof M and the associated infrastructure

4dcba4d

lschaefer-sugarcrm requested review from a user and mmarum-sugarcrm March 28, 2018 14:21

mmarum-sugarcrm requested changes Mar 28, 2018

View reviewed changes

lschaefer-sugarcrm force-pushed the integration-tests branch from 979640e to ba04b62 Compare March 29, 2018 13:54

lschaefer-sugarcrm force-pushed the integration-tests branch from ba04b62 to e4bf826 Compare April 2, 2018 12:44

Updating PHPUnit automation scripts based on code review

7f9c771

Update config_override Update GIT_HUB to GITHUB Update config_si Fix headers error Remove sudo from calls to get install to work on local machine

lschaefer-sugarcrm force-pushed the integration-tests branch from d1d3d86 to 7f9c771 Compare April 2, 2018 16:34

glevine suggested changes Apr 2, 2018

View reviewed changes

Update PHPUnit tests & after save hook based on code review

c27d7f6

lschaefer-sugarcrm force-pushed the integration-tests branch from f47d2f8 to c27d7f6 Compare April 3, 2018 14:12

lschaefer-sugarcrm mentioned this pull request Apr 3, 2018

Add support for integration tests #43

Open

lschaefer-sugarcrm mentioned this pull request Apr 3, 2018

Get builds from store #44

Merged

glevine suggested changes Apr 5, 2018

View reviewed changes

glevine reviewed Apr 5, 2018

View reviewed changes

mmarum-sugarcrm requested changes Apr 5, 2018

View reviewed changes

mmarum-sugarcrm reviewed Apr 5, 2018

View reviewed changes

mmarum-sugarcrm requested changes Apr 5, 2018

View reviewed changes

lschaefer-sugarcrm force-pushed the integration-tests branch from 60cac82 to a02378b Compare April 6, 2018 13:59

Improvements to gradebook tests

7ae12eb

lschaefer-sugarcrm force-pushed the integration-tests branch from a02378b to 7ae12eb Compare April 6, 2018 14:46

glevine reviewed Apr 6, 2018

View reviewed changes

glevine approved these changes Apr 6, 2018

View reviewed changes

Removing Sugar PHPUnit test run

6a0b4b4

Update README to reflect Sugar PHPUnit tests not run as part of build

lschaefer-sugarcrm force-pushed the integration-tests branch from 824a9f7 to 6a0b4b4 Compare April 9, 2018 14:32

lschaefer-sugarcrm merged commit 4e8f138 into master Apr 9, 2018

lschaefer-sugarcrm deleted the integration-tests branch April 9, 2018 14:56


		use Sugarcrm\Sugarcrm\custom\gradebook_fake\RecordManager;

		class StudentGradebookJob implements RunnableSchedulerJob {

		@@ -0,0 +1,19 @@
		#!/usr/bin/env bash

		# This script runs the Sugar PHPUnit tests. It assumes the Sugar Docker stack has already been started and that

Integration tests #38

Integration tests #38

Conversation

lschaefer-sugarcrm commented Mar 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lschaefer-sugarcrm commented Mar 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glevine commented Apr 2, 2018 • edited Loading

lschaefer-sugarcrm commented Apr 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmarum-sugarcrm left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lschaefer-sugarcrm commented Apr 6, 2018

lschaefer-sugarcrm commented Apr 6, 2018

Choose a reason for hiding this comment

lschaefer-sugarcrm commented Apr 9, 2018

glevine commented Apr 2, 2018 •

edited

Loading

mmarum-sugarcrm left a comment •

edited

Loading