Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPII-1987: Implement data loaders to load preferences and authorization test data into CouchDB #503

Closed
wants to merge 43 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@cindyli
Copy link
Contributor

cindyli commented Feb 15, 2017

cindyli added some commits Nov 11, 2016

GPII-1987: Dropped the use of kettle.dataSource.CouchDB by using kett…
…le.dataSource.URL because for creating and loading data into CouchDB, a pre-fetch feature provided by kettle.dataSource.CouchDB for checking on _revision value is not needed.
GPII-1987: Splitted authDataLoader.js into 2 js files that one contai…
…ns the actual implementation of the auth loader component and the other is to call the loader component to perform the loading so that the latter can be enhanced to accept an configuration.
GPII-1987: Uses avtar's fix on ansible-preferences-server that allows…
… the GPII production VM to start properly.

@cindyli cindyli requested a review from amb26 Feb 15, 2017

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Feb 22, 2017

CI job passed.

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Feb 22, 2017

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

@gtirloni

This comment has been minimized.

Copy link
Contributor

gtirloni commented Feb 22, 2017

ok to test

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Feb 22, 2017

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

@gtirloni

This comment has been minimized.

Copy link
Contributor

gtirloni commented Feb 22, 2017

ok to test

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Feb 22, 2017

CI job passed.

@amb26

This comment has been minimized.

Copy link
Member

amb26 commented Feb 23, 2017

When run on Windows. each test file still leaves several couchDB databases behind in /Temp, as well as the tests hanging in a characteristic way on attempted cleanup, although they do pass in the end:

02:48:01.927: Sending a GET request to: /auth on port 1234
02:48:06.946: Test case listener has not responded after 5000ms - at sequence pos 6 of 6 sequence element {
"event": "{gpii.test.pouch.environment}.events.onCleanupComplete",
"listener": "fluid.log",
"args": [
"Database cleanup complete"
]
} of fixture Testing the successful loading
02:48:14.682: ERROR: Unable to remove express-pouchdb base directory...
{
"errno": -4082,
"code": "EBUSY",
"syscall": "unlink",
"path": "C:\Users\Bosmon\AppData\Local\Temp\1qmu4mpp-154\auth\LOCK"
}
02:48:14.688: Database cleanup complete
02:48:14.692: Express stopped...
02:48:14.721: Successfully queued test Testing the non-existence of data files
02:48:14.730: jq: Test concluded - Module "Testing Auth Data Loader - success" Test name "Testing the successful loading": 3/3 passed - PASS
02:48:14.755: express pouchdb instance '1qmu4mpp-313' initalizing...
02:48:14.756: Creating directory 'C:\Users\Bosmon\AppData\Local\Temp\1qmu4mpp-313' for express pouchdb instance '1qmu4mpp-313'...
02:48:14.772: express baseDir: 'C:\Users\Bosmon\AppData\Local\Temp\1qmu4mpp-313'...
02:48:14.776: Express server listening on port 1234
02:48:14.787: Express started...
Error at loading the auth data. Error details: Data file(s) not found in the file system: E:/source/gits/gpii/node_modules/universal/tests/nonExistent.json
02:48:14.838: Sending a GET request to: /auth on port 1234
02:48:19.970: Test case listener has not responded after 5000ms - at sequence pos 7 of 7 sequence element {
"event": "{gpii.test.pouch.environment}.events.onCleanupComplete",
"listener": "fluid.log",
"args": [
"Database cleanup complete"
]
} of fixture Testing the non-existence of data files
02:48:27.727: ERROR: Unable to remove express-pouchdb base directory...
{
"errno": -4082,
"code": "EBUSY",
"syscall": "unlink",
"path": "C:\Users\Bosmon\AppData\Local\Temp\1qmu4mpp-313\_users\000003.log"
}
02:48:27.730: Database cleanup complete
02:48:27.733: Express stopped...
02:48:27.745: jq: Test concluded - Module "Testing Auth Data Loader - error" Test name "Testing the non-existence of data files": 2/2 passed - PASS
02:48:27.748: jq: ***************
02:48:27.762: jq: All tests concluded: 5/5 total passed in 27102ms - PASS
02:48:27.763: jq: ***************

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Feb 23, 2017

CI job passed.

GPII-1987: Switched to use gpii-ops/ansible-preferences-server master…
… branch since avtar's pull request has been merged in.
@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Feb 24, 2017

CI job passed.

@cindyli

This comment has been minimized.

Copy link
Contributor Author

cindyli commented Feb 27, 2017

@amb26, as per the discussion in the IRC channel today, @the-t-in-rtf has added a posttest step in gpii-pouchdb to handle the issue that pouchdb-created-temp-files were not removed due to their lock states. This change is already included in this pull request.

The posttest step will automatically run and do the final cleanup if tests were ran by issuing the command npm test. However, if tests were ran using other means like "node all-tests.js", this posttest step will need to be manually issued.

Do you think README should be updated to include this explanation?

@the-t-in-rtf

This comment has been minimized.

Copy link
Member

the-t-in-rtf commented Feb 28, 2017

@cindyli, the current wording in the README suggests using npm test to run the tests, and does not mention the node option. I'd imagine devs might want to call the tests directly for debugging purposes, but I would assume that someone who can work with node and a debugger knows to read the package.json file and figure out the command behind npm test. On balance, I'd say you can probably omit it from the README, but put a comment in the file itself.

@the-t-in-rtf

This comment has been minimized.

Copy link
Member

the-t-in-rtf commented Feb 28, 2017

On a side note, I think we need to provide a means for running the browser tests in multiple browsers with a single command, and to include that command as part of npm test as well.

In my own work I've used script definitions like:

"scripts": {
  "tests:node": "istanbul cover tests/all-tests.js",
  "tests:browser": "testem ci",
  "tests": "npm run tests:node && npm run tests:browser"
}

Note that my example also includes code coverage for the node tests, which is also currently missing. If you both are in agreement, I'm happy to handle both in a small PR of my own against universal, as I'll be doing similar work in a few projects in the next week or so.

@cindyli

This comment has been minimized.

Copy link
Contributor Author

cindyli commented Feb 28, 2017

@the-t-in-rtf, after the IRC discussion, please go ahead to issue the pull request for npm test. Note that universal has various grunt tasks for running different set of tests.

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Mar 1, 2017

CI job passed.

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented Apr 28, 2017

CI job passed.

@the-t-in-rtf

This comment has been minimized.

Copy link
Member

the-t-in-rtf commented May 1, 2017

Hi, @cindyli. The PR to add code coverage (and move away from Grunt scripts) can be found here: #508

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

gpii-bot commented May 1, 2017

CI job passed.

@cindyli

This comment has been minimized.

Copy link
Contributor Author

cindyli commented May 1, 2017

@amb26, this pull request has been updated to use the latest gpii-pouchdb release and ready for another review.

@amb26

This comment has been minimized.

Copy link
Member

amb26 commented Oct 23, 2018

@cindyli - I'm assuming this pull can be closed given the change in our requirements for data loading?

@cindyli

This comment has been minimized.

Copy link
Contributor Author

cindyli commented Oct 23, 2018

Closed as to the requirement change on data loading that is undergoing via #626 and #692.

@cindyli cindyli closed this Oct 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.