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.

Sign up for GitHub

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

Jump to bottom

Skip imcompatible tests #1420

Merged
merged 4 commits into from
Jul 18, 2017
Merged

Skip imcompatible tests #1420

merged 4 commits into from
Jul 18, 2017

Conversation

jannyHou
Copy link
Contributor

@jannyHou jannyHou commented Jul 14, 2017 •
edited
Loading

@jannyHou jannyHou mentioned this pull request Jul 14, 2017
Merged
@jannyHou jannyHou changed the title Skip imcompatible tests [WIP]Skip imcompatible tests Jul 14, 2017
jannyHou and others added 2 commits July 14, 2017 15:14
@jannyHou
Skip replace
34e6a57
@virkt25
Fix tests failing when overridden for cloudant.
3f2a370 
Fixed short-circuiting of test cases preventing then from being run
standalone.
@jannyHou jannyHou force-pushed the cloudant/crud-test branch from d1f751a to 3f2a370 Compare July 18, 2017 17:40
@jannyHou jannyHou changed the title [WIP]Skip imcompatible tests Skip imcompatible tests Jul 18, 2017
jannyHou
jannyHou commented Jul 18, 2017
View reviewed changes
test/manipulation.test.js Outdated
@@ -1822,6 +1825,11 @@ describe('manipulation', function() {
});

it('should allow delete(id) - success', function(done) {
// Suspect race condition happens somewhere
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove the comments. Fixed now.

jannyHou
jannyHou commented Jul 18, 2017
View reviewed changes
test/manipulation.test.js Outdated
@@ -1837,6 +1845,7 @@ describe('manipulation', function() {
});

it('should allow delete(id) - fail', function(done) {
// Same reason, please check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto

ssh24
ssh24 approved these changes Jul 18, 2017
View reviewed changes
Copy link
Contributor

@ssh24 ssh24 left a comment

Choose a reason for hiding this comment

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

LGTM

virkt25
virkt25 reviewed Jul 18, 2017
View reviewed changes
test/manipulation.test.js Outdated
@@ -1860,8 +1863,7 @@ describe('manipulation', function() {
});
});

bdd.itIf(connectorCapabilities.supportStrictDelete !== false, 'should allow delete(id) - ' +
'fail with error', function(done) {
it('should allow delete(id) - ' + 'fail with error', function(done) {
Copy link
Contributor

@virkt25 virkt25 Jul 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

supportStrictDelete flag shouldn't be removed along with the cloudantCompatibility flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops

virkt25
virkt25 reviewed Jul 18, 2017
View reviewed changes
Copy link
Contributor

@virkt25 virkt25 left a comment

Choose a reason for hiding this comment

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

supportStrictDelete needs to be stay.

jannyHou
jannyHou commented Jul 18, 2017
View reviewed changes
test/manipulation.test.js Outdated
if (err) return done(err);
info.should.have.property('count', 5);
Person.find({where: filterBrett}, function(err, people) {
bdd.itIf(connectorCapabilities.updateWithoutId !== false &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, hold on, IIRC this one should be a good test for cloudant.

b-admike
b-admike approved these changes Jul 18, 2017
View reviewed changes
Copy link
Contributor

@b-admike b-admike left a comment

Choose a reason for hiding this comment

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

🚢🇮🇹

@jannyHou jannyHou force-pushed the cloudant/crud-test branch from 0b7f55d to 4976c10 Compare July 18, 2017 18:56
@jannyHou jannyHou merged commit 6a1b555 into master Jul 18, 2017
@bajtos bajtos mentioned this pull request Jul 20, 2017
Merged
2 tasks
kjdelisle pushed a commit that referenced this pull request Jul 26, 2017
3.9.3
bebac1e 
 * Catch errors using cb (loay)
 * Rename getAsync() methods to find() and get() (Jürg Lehni)
 * #1386 Allow empty values when allowBlank is true (Simo Moujami)
 * Skip imcompatible tests (#1420) (Janny)
 * Run juggler tests for Cloudant (#1414) (Janny)
@jannyHou jannyHou deleted the cloudant/crud-test branch April 12, 2019 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@virkt25 virkt25 virkt25 left review comments

@ssh24 ssh24 ssh24 approved these changes

@b-admike b-admike b-admike approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jannyHou @virkt25 @b-admike @ssh24