Skip to content

GPII-3619: Convert universal tests to use CouchDB instead of PouchDB.#728

Merged
amb26 merged 79 commits intoGPII:masterfrom
the-t-in-rtf:GPII-3619
Feb 20, 2019
Merged

GPII-3619: Convert universal tests to use CouchDB instead of PouchDB.#728
amb26 merged 79 commits intoGPII:masterfrom
the-t-in-rtf:GPII-3619

Conversation

@the-t-in-rtf
Copy link
Member

Replaces all node-side usages of PouchDB with gpii-couchdb-test-harness. See GPII-3619 for details. Fixes bugs discovered in using CouchDB for the tests, such as GPII-3628.

Also replaces a lot of legacy test-building code with modern sequence elements and grades.

…cks into launch handler block. Removed duplicate capability definition after review of logs.
…s and disabled TypingEcho, as it is known to cause problems.
…opefully avoid cross-pollenating data when values are unset.
… Fixed typo in TextViewerFontFaceName inverse capability transform.
# Conflicts:
#	gpii/node_modules/testing/src/TestUtils.js
#	tests/UntrustedUserLogonRequestTests.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "we shouldn't use gradeNames"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, I didn't write that, I just moved it. Here it is in master.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to bin it if it doesn't seem like a useful note for the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please bin - we can consider whatever intent was behind the comment is enacted moving to sequence grades

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, will commit once I fix the broken prefs server tests.

debounceTimeoutComplete: null
},
expectedModelChanges: {
proximityTriggeredLogon: [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this model makes a lot more sense

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad, I was shooting for something clearer and reliable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray commented out definition

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that was left over from a fairly serious goof with a broken environment where that grade was required.

listener: "fluid.identity"
}
]
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance we could lose the massive formatting diffs in this file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated above, I really really dislike not indenting multi-element arrays, and indenting them more cleanly made it easier to do the review work required for this pull. I think of it as something akin to alphabetising a previously unordered list of solutions registry entries. It made it easier to perform the work in question today, and hopefully will make it easier to maintain things in the future, even if it does bloat the pull.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In follow up work I have noticed additional small multi-element arrays and left them alone for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, don't!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, thanks, fixed.

@the-t-in-rtf
Copy link
Member Author

All of my "done" and "fixed" comments above are pending a local test run and will soon be committed.

@the-t-in-rtf
Copy link
Member Author

Rather than doing a double tap, I decided to commit so that my changes were visible, and then look at the results in CI.

@gpii-bot
Copy link

CI job failed: https://ci.gpii.net/job/universal-tests/1543/

@the-t-in-rtf
Copy link
Member Author

Looks like the commented out configurations in the preferences server tests are actually broken, fixing those now.

@the-t-in-rtf
Copy link
Member Author

the-t-in-rtf commented Feb 18, 2019

I fixed one of the commented out tests easily enough, but I ended up uncovering a bug with the "without db" variants that I wrote up as GPII-3733. Those tests are commented out for now.

@gpii-bot
Copy link

CI job passed: https://ci.gpii.net/job/universal-tests/1546/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should read CouchDB

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should read CouchDB

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anomalous spacing appeared here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's aligned in the way I often do, and is consistent with itself, but I see that the rest of the test defs follow another style. I'll change it.

@amb26
Copy link
Member

amb26 commented Feb 19, 2019

Please do a search throughout the project for "Pouch" and remove all stray references - I find 29 still remaining

@the-t-in-rtf
Copy link
Member Author

Sorry, I think my last search for all references to "pouch" may have either been case-sensitive or otherwise incomplete. Will fix it now.

@the-t-in-rtf
Copy link
Member Author

@amb26, this should be ready to review again.

@gpii-bot
Copy link

CI job failed: https://ci.gpii.net/job/universal-tests/1548/

@the-t-in-rtf
Copy link
Member Author

Looks like the merge with upstream brought in "new old world" tests that I need to quickly update:

10:34:38.539: FATAL ERROR: Uncaught exception: gpii.test.bootstrapServer is not a function
TypeError: gpii.test.bootstrapServer is not a function
at Object. (/home/vagrant/sync/universal/tests/SuppressHttpEndpointsTests.js:20:11)

@gpii-bot
Copy link

CI job passed: https://ci.gpii.net/job/universal-tests/1549/

@amb26 amb26 merged commit ec90b2f into GPII:master Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments