Switches to local shared-libs #5684

dianabarsan · 2019-05-22T07:29:53Z

Description

Changes transitions error reporting to not stringify errors.
Changes lineage lib to not require pouchdb and run integration tests with memory adapter, to avoid building leveldown every time we install or test the library.
Changes packages for all apps so shared-libs are local again.
Adds jasmine reporter to improve e2e tests outputs.

#5694

Code review items

Readable: Concise, well named, follows the style guide, documented if necessary.
Documented: Configuration and user documentation on medic-docs
Tested: Unit and/or e2e where appropriate
Internationalised: All user facing text
Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in Changes.md.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

dianabarsan · 2019-05-22T20:52:57Z

Gruntfile.js

@@ -815,7 +815,7 @@ module.exports = function(grunt) {
  // Build tasks
  grunt.registerTask('install-dependencies', 'Update and patch dependencies', [
    'exec:undo-patches',
-    'exec:npm-ci-shared-libs',
+    'exec:npm-ci-shared-libs:production',


Doing this to not pack shared-libs dev dependencies.

I think this command is only ever called from here, so I think we should just hardcode --production into the command above.

dianabarsan · 2019-05-22T20:56:30Z

package.json

@@ -50,6 +50,7 @@
    "grunt-sass": "^3.0.2",
    "grunt-text-replace": "^0.4.0",
    "grunt-xmlmin": "^0.1.8",
+    "jasmine-spec-reporter": "^4.2.1",


Makes e2e jasmine spec reporting look pretty:
https://travis-ci.org/medic/medic/jobs/535893166#L3633 - successfull build
https://travis-ci.org/medic/medic/jobs/535888193#L3682 - deliberately failed build

dianabarsan · 2019-05-22T20:59:27Z

shared-libs/memdown/src/memdown-medic.js

-const PouchDB = require('pouchdb').defaults({ db: memdown });
+const PouchDB = require('pouchdb-core');
+PouchDB.plugin(require('pouchdb-adapter-memory'));
+PouchDB.plugin(require('pouchdb-mapreduce'));


Runs integration tests with memory adapter instead of leveldown, so we don't need to build leveldown every time we install or test this dependency. Shaves ~3 minutes off the Travis build job.

Wow. Awesome!

dianabarsan · 2019-05-22T21:02:07Z

shared-libs/transitions/src/transitions/index.js

@@ -238,7 +238,7 @@ const finalize = ({ change, results }, callback) => {
    // todo: how to handle a failed save? for now just
    // waiting until next change and try again.
    if (err) {
-      logger.error(`error saving changes on doc ${change.id} seq ${change.seq}: ${JSON.stringify(err)}`);
+      logger.error(`error saving changes on doc ${change.id} seq ${change.seq}: %o`, err);


console.log(JSON.stringify(new Error('you would expect something else, right?'))); => {}

dianabarsan · 2019-05-23T16:01:03Z

Gruntfile.js

@@ -962,7 +962,6 @@ module.exports = function(grunt) {
    'static-analysis',
    'build',
    'build-admin',
-    'install-dependencies',


I couldn't find a good reason to install dependencies the second time.

Totally. Must've been a bug...

dianabarsan · 2019-05-23T16:14:02Z

@garethbowen could you please have a look?
I put a bunch of other small stuff in, but I could separate them if you want.

garethbowen

Awesome. So many improvements in a single PR! I've made a few minor comments inline.

I'm a little concerned about destabilising 3.5.0 so near the finish line. Do you think it's worth holding off merging until 3.6.0?

I think we should have an issue created to track this. I think it could use a little AT to make sure the release works as expected. What do you think?

garethbowen · 2019-05-23T21:50:26Z

Gruntfile.js

          return getSharedLibDirs()
            .map(
              lib =>
                `echo Installing shared library: ${lib} &&
-                  (cd shared-libs/${lib} && npm ci)`
+                  (cd shared-libs/${lib} && npm ci ${production && '--production' || ''})`


I think a ternary would be clearer, eg: ${production ? '--production' : ''}

garethbowen · 2019-05-23T21:52:19Z

Gruntfile.js

@@ -962,7 +962,6 @@ module.exports = function(grunt) {
    'static-analysis',
    'build',
    'build-admin',
-    'install-dependencies',


Totally. Must've been a bug...

garethbowen · 2019-05-23T21:53:36Z

Gruntfile.js

@@ -815,7 +815,7 @@ module.exports = function(grunt) {
  // Build tasks
  grunt.registerTask('install-dependencies', 'Update and patch dependencies', [
    'exec:undo-patches',
-    'exec:npm-ci-shared-libs',
+    'exec:npm-ci-shared-libs:production',


I think this command is only ever called from here, so I think we should just hardcode --production into the command above.

garethbowen · 2019-05-23T21:56:02Z

api/package.json

    "async": "^2.6.1",
-    "bikram-sambat": "^1.5.0",
+    "bikram-sambat": "1.5.0",
+    "bikram-sambat-bootstrap": "1.4.2",


These versions should have a ^, right?

garethbowen · 2019-05-23T21:56:36Z

package.json

@@ -50,6 +50,7 @@
    "grunt-sass": "^3.0.2",
    "grunt-text-replace": "^0.4.0",
    "grunt-xmlmin": "^0.1.8",
+    "jasmine-spec-reporter": "^4.2.1",


garethbowen · 2019-05-23T21:59:44Z

shared-libs/memdown/src/memdown-medic.js

-const PouchDB = require('pouchdb').defaults({ db: memdown });
+const PouchDB = require('pouchdb-core');
+PouchDB.plugin(require('pouchdb-adapter-memory'));
+PouchDB.plugin(require('pouchdb-mapreduce'));


Wow. Awesome!

garethbowen · 2019-05-23T22:01:31Z

shared-libs/message-utils/package.json

-    "object-path": "^0.11.4"
-  },
-  "dependencies": {
+    "object-path": "^0.11.4",


This has moved most of the libs from devDependencies to dependencies - were they previously in the wrong place?

Yep. Because I'm installing them with --production, I needed them in the proper place (they're not dev dependencies).

garethbowen · 2019-05-23T22:03:11Z

shared-libs/transitions/src/transitions/index.js

@@ -238,7 +238,7 @@ const finalize = ({ change, results }, callback) => {
    // todo: how to handle a failed save? for now just
    // waiting until next change and try again.
    if (err) {
-      logger.error(`error saving changes on doc ${change.id} seq ${change.seq}: ${JSON.stringify(err)}`);
+      logger.error(`error saving changes on doc ${change.id} seq ${change.seq}: %o`, err);


garethbowen · 2019-05-23T22:05:06Z

tests/base.conf.js

+      jasmineNodeOpts: {
+        // makes default jasmine reporter not display dots for every spec
+        print: () => {}
+      },


You don't like the dots?

Occasionally it's useful to know that the tests are still running.

Can we print the name of the test that's being executed instead of a dot? Potentially we could only print the name if in debug mode?

I don't like the dots, the dots along with the names look stange - the default reporter displays a . for a successful test and an F for a failure.
Together they look like this:

I do like the names, and I'd like to see them in my Travis builds.
This new reporter offers plenty of customization. Would you rather the name of the test be displayed at start and then a separate message upon success/failure?

No that's perfect, without the dots! It's fairly easy to work out which test is running if you can see which test has just completed so leave it as is.

garethbowen

Great stuff!

The code is good to go. Please raise a new issue covering your changes and schedule it for 3.6.0 (in AT) and it can be merged once 3.5.0 is out the door.

dianabarsan · 2019-05-27T15:53:48Z

@garethbowen
Thanks for the review!
I added a small update to add spec duration to the reporter (like in the image I posted) and to add watches for shared-libs folders to the nodemon api and sentinel commands. Could you please review that as well? Thanks again!
Also, I created the issue and scheduled it in 3.6.

garethbowen

The duration looks great. Good work!

dianabarsan force-pushed the fix-error-logging branch from afc5b4b to f95b9d0 Compare May 22, 2019 16:07

dianabarsan commented May 22, 2019

View reviewed changes

dianabarsan changed the title ~~Dependencies juggling~~ Switches to local shared-libs May 23, 2019

dianabarsan force-pushed the fix-error-logging branch from e9be545 to c595ec2 Compare May 23, 2019 10:01

dianabarsan commented May 23, 2019

View reviewed changes

dianabarsan requested a review from garethbowen May 23, 2019 16:14

garethbowen requested changes May 23, 2019

View reviewed changes

garethbowen approved these changes May 26, 2019

View reviewed changes

dianabarsan requested a review from garethbowen May 27, 2019 15:54

garethbowen approved these changes May 27, 2019

View reviewed changes

kennsippell mentioned this pull request Jun 4, 2019

SIH outbound push rebased onto local shared-libs #5709

Closed

garethbowen mentioned this pull request Jun 5, 2019

Support Node 12 #5607

Closed

dianabarsan added 13 commits June 20, 2019 11:11

have a go.

9e92a22

Don't stringify errors.

8a2d33b

No leveldown & prettier e2e.

1ae7f3c

Updates to lineage dependencies.

27e8130

Update some dependencies and better gruntfile tasks.

ca3987d

Updates many dependencies.

954655c

Update jasmine/protractor settings.

636fc18

Update dependencies again.

cf41daf

Add explanatory comment.

952d4aa

Deliberately fail this test.

e76bd9f

revert deliberate fail.

3fede27

Bundle depdendencies.

9116834

Fix dependencies and update task.

a27a691

Add e2e duration and api/sentinel watches for shared-libs

bceef58

garethbowen force-pushed the fix-error-logging branch from fd31cb7 to bceef58 Compare June 19, 2019 23:18

garethbowen merged commit 704e9df into master Jun 19, 2019

garethbowen deleted the fix-error-logging branch June 19, 2019 23:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switches to local shared-libs #5684

Switches to local shared-libs #5684

dianabarsan commented May 22, 2019 •

edited by garethbowen

dianabarsan May 22, 2019

garethbowen May 23, 2019

dianabarsan May 22, 2019

garethbowen May 23, 2019

dianabarsan May 22, 2019 •

edited

garethbowen May 23, 2019

dianabarsan May 22, 2019

garethbowen May 23, 2019

dianabarsan May 23, 2019

garethbowen May 23, 2019

dianabarsan commented May 23, 2019

garethbowen left a comment

garethbowen May 23, 2019

garethbowen May 23, 2019

garethbowen May 23, 2019

garethbowen May 23, 2019

garethbowen May 23, 2019

garethbowen May 23, 2019

garethbowen May 23, 2019

dianabarsan May 24, 2019

garethbowen May 26, 2019

garethbowen May 23, 2019

garethbowen May 23, 2019

dianabarsan May 24, 2019 •

edited

garethbowen May 26, 2019

garethbowen left a comment

dianabarsan commented May 27, 2019 •

edited

garethbowen left a comment

Switches to local shared-libs #5684

Switches to local shared-libs #5684

Conversation

dianabarsan commented May 22, 2019 • edited by garethbowen

Description

Code review items

License

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

dianabarsan May 22, 2019 • edited

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

dianabarsan commented May 23, 2019

garethbowen left a 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

dianabarsan May 24, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garethbowen left a comment

Choose a reason for hiding this comment

dianabarsan commented May 27, 2019 • edited

garethbowen left a comment

Choose a reason for hiding this comment

dianabarsan commented May 22, 2019 •

edited by garethbowen

dianabarsan May 22, 2019 •

edited

dianabarsan May 24, 2019 •

edited

dianabarsan commented May 27, 2019 •

edited