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

juggler-v3-v4 shared test #206

Merged
merged 1 commit into from Jun 21, 2019

Conversation

Projects
None yet
5 participants
@agnes512
Copy link
Contributor

commented Jun 17, 2019

Description

At the moment, our connectors are installing juggler 3.x to run the test suite. When we make a change to juggler@3, cis-jenkins detects downstream dependency and triggers CI build of connectors with the modified juggler version. When we make a change to juggler's master, no downstream builds are triggered.

In the past, we were maintaining multiple branches to test connectors against different juggler versions. E.g. loopback-2.x containing code from "master" but installing juggler 2.x for testing. This is rather impractical, because we don't have bandwidth to update those other branches with changes made to master.

In this pull request, I am reworking connector test setup to execute tests from both juggler versions 3.x and 4.x, which are triggered in ./test.js.

Drop Node.js 6 CI tests as Juggler v4 does not support it. Also update eslint to the latest version (see those 2 PRs below).

Related issues

References issues

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide


/** from couldant.connection.test */
describe('cloudant connection', function() {

This comment has been minimized.

Copy link
@bajtos

bajtos Jun 18, 2019

Member

I think this should stay in cloudant.connection.test.

The purpose of juggler-v*/test.js is to execute the shared test suite provided by juggler. Connector-specific tests are staying in test/* files and are executed against a single juggler version only. At least that have been our approach so far.

return global.resetDataSourceClass();
});
});
// keep this part?
describe('cloudant imported features', function() {

This comment has been minimized.

Copy link
@bajtos

bajtos Jun 18, 2019

Member

Let's remove this describe line, we already have describe(name,...) above - that should be enough.

It's also crucial to call require('loopback-datasource-juggler/test/*'); from inside describe(name,...) block!

describe(name, function() {
  before(function() {
    IMPORTED_TEST = true;
    return global.resetDataSourceClass(juggler.DataSource);
  });
	
  after(function() {
    IMPORTED_TEST = false;
    return global.resetDataSourceClass();
  });

  require('loopback-datasource-juggler/test/include.test.js');
  require('loopback-datasource-juggler/test/common.batch.js');
});

process.env.COUCHDB2_TEST_SKIP_INIT = true;

describe('cloudant automigrate.test - imported from couchdb2', function() {

This comment has been minimized.

Copy link
@bajtos

bajtos Jun 18, 2019

Member

The tests imported from loopback-connector-couchdb2 should stay in test/imported.test.js, they are not part of deps/juggler-v* test suite.

});

after(function() {
return global.resetDataSourceClass();

This comment has been minimized.

Copy link
@bajtos

bajtos Jun 18, 2019

Member

To make this work, you need to modify test/init.js, add resetDataSourceClass helper and modify getDataSource to allow configuration of the data-source class used. See https://github.com/strongloop/loopback-connector-mongodb/pull/519/files#diff-40dfb1c53004f1488cbe6eb300b286dd

@agnes512

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

@slnode test please

@jannyHou jannyHou referenced this pull request Jun 19, 2019

Merged

test: add property index to support cloudant #1750

2 of 2 tasks complete
@jannyHou

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

strongloop/loopback-datasource-juggler#1750 should fix the 2 failures for the tests with juggler 4.x

@agnes512 agnes512 force-pushed the build/test-juggler-v3-and-v4 branch 4 times, most recently from 80c18fc to 8d1e93b Jun 19, 2019

@agnes512 agnes512 marked this pull request as ready for review Jun 19, 2019

@agnes512

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

@slnode test please

});
require('loopback-datasource-juggler/test/include.test.js');
require('loopback-datasource-juggler/test/common.batch.js');
});

This comment has been minimized.

Copy link
@bajtos

bajtos Jun 20, 2019

Member

Please add a TODO comment to remind us that we should eventually include the following tests too:

  • loopback-datasource-juggler/test/default-scope.test.js
  • loopback-datasource-juggler/test/persistence-hooks.suite.js

See https://github.com/strongloop/loopback-connector-mongodb/blob/master/deps/juggler-v4/test.js

"eslint-config-loopback": "^4.0.0",
"loopback-datasource-juggler": "^3.0.0",
"eslint": "^5.1.0",
"eslint-config-loopback": "^10.0.0",

This comment has been minimized.

Copy link
@bajtos

bajtos Jun 20, 2019

Member

Update of eslint-config-loopback causes a lot of reformatting (adds a lot of insignificant white-space changes), which makes it difficult to see what changes are relevant to the introduction of the shared tests.

Please open a new pull request to update eslint config to the latest version & fix linting problems. Once that PR is landed, rebase this PR on top of the new master so that it contains only changes relevant to the introduction of the shared tests.

@agnes512 agnes512 force-pushed the build/test-juggler-v3-and-v4 branch 3 times, most recently from b364cf9 to 98b11e3 Jun 20, 2019

@agnes512 agnes512 force-pushed the build/test-juggler-v3-and-v4 branch from b23840b to 5fa0e51 Jun 21, 2019

@b-admike
Copy link
Member

left a comment

👍

@agnes512 agnes512 force-pushed the build/test-juggler-v3-and-v4 branch from 5fa0e51 to e0539a3 Jun 21, 2019

@nabdelgadir
Copy link

left a comment

👏

@agnes512 agnes512 merged commit 0ffcd3c into master Jun 21, 2019

9 checks passed

Commit Linter commits are all properly formatted
Details
PR Linter PR is up to date
Details
[cis-jenkins] PR Builder Build finished.
Details
[cis-jenkins] x64 && linux && nvm && docker-in-docker,10 Success! (e0539a3)
Details
[cis-jenkins] x64 && linux && nvm && docker-in-docker,8 Success! (e0539a3)
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
security/snyk - package.json (StrongLoop) No new issues
Details

@delete-merged-branch delete-merged-branch bot deleted the build/test-juggler-v3-and-v4 branch Jun 21, 2019

@bajtos
Copy link
Member

left a comment

👏

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.