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

Enable Version Vectors #3329

Merged
merged 5 commits into from
Nov 18, 2024
Merged

Enable Version Vectors #3329

merged 5 commits into from
Nov 18, 2024
+112 −83

Conversation

velicuvlad
Copy link
Contributor

@velicuvlad velicuvlad commented Sep 23, 2024 •
edited
Loading

  • CBL-6215
  • CBL-6219
  • CBL-6223
  • removed generation API which was used for tests
  • LiteCore 4.0.0-26
  • removed test for -1 and 0 LIMIT boundaries

@velicuvlad velicuvlad marked this pull request as ready for review September 23, 2024 15:06
@velicuvlad
Copy link
Contributor Author

velicuvlad commented Sep 23, 2024 •
edited
Loading

EE Jenkins failing with the below, from LiteCore RevTree.cc

[2024-09-23T15:09:15.474Z] Testing failed:

[2024-09-23T15:09:15.474Z] 	No member named 'any_of' in namespace 'std::ranges'

[2024-09-23T15:09:15.474Z] 	No member named 'sort' in namespace 'std::ranges'

[2024-09-23T15:09:15.474Z] 	Testing cancelled because the build failed.

@pasin
Copy link
Contributor

pasin commented Oct 3, 2024

Jim has a new mac mini that can install a new XCode that supports C++ 20. You can check with him about the agent name.

@velicuvlad
Copy link
Contributor Author

@pasin I disabled testReplicatedDocument as per the symbol not being found for c4str, as discussed on slack.

@velicuvlad velicuvlad requested a review from pasin October 31, 2024 16:02
pasin
pasin requested changes Oct 31, 2024
View reviewed changes
Copy link
Contributor

@pasin pasin left a comment

Choose a reason for hiding this comment

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

  1. As discussed on slack, use C4STR or FLSTR instead.

  2. do you know why all SG tests failed?

  3. LiteCore is using this agent for iOS PR Validation. iOS can do the same here.
    https://github.com/couchbase/couchbase-lite-core/blob/master/Jenkinsfile#L19

@velicuvlad
Copy link
Contributor Author

velicuvlad commented Nov 12, 2024 •
edited
Loading

  1. it is known that Unnest and Array index are not completely merged in LiteCore 4.0.0-26 - testCreateArrayIndexWithPath fails as a result of that. I see this on local as well. No ticket as I am yet to figure why it is only this one is invalid n1ql index expression. At the same time, we expect another LiteCore update.
  2. SGW tests to be fixed and updated in a separate PR - I have a separate ticket for that

With the above in mind, this is ready to be merged.

pasin
pasin reviewed Nov 12, 2024
View reviewed changes
Objective-C/Tests/DocumentTest.m Outdated
Assert(doc._getRevisionHistory);
// Fails from LiteCore,only 1 rev, no rev history
// doc = [[defaultCollection documentWithID: @"doc1" error: &err] toMutable];
// Assert(doc._getRevisionHistory);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this expected or failed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one change slipped when I resolved conflicts... by default, we only get current rev from LiteCore for a doc, so that would've failed. Now we grab using correct content flag, only for the revHist functionality

Swift/Document.swift Outdated
@@ -33,6 +33,10 @@ public class Document : DictionaryProtocol, Equatable, Hashable, Sequence {
return impl.revisionID
}

public var timestamp: Double {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be NSTimeInterval instead of Double?

Copy link
Contributor

Choose a reason for hiding this comment

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

TimeInterval specifically for Swift.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, will change to TimeInterval

Objective-C/Tests/DatabaseTest.m Outdated
@@ -598,70 +598,72 @@ - (void) testSavePurgedDoc {

#pragma mark Save Conflict Handler

- (NSTimeInterval) checkTimestampUpdated:(CBLDocument*) doc
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, removed

pasin
pasin requested changes Nov 14, 2024
View reviewed changes
Swift/Document.swift Outdated
@@ -33,6 +33,10 @@ public class Document : DictionaryProtocol, Equatable, Hashable, Sequence {
return impl.revisionID
}

public var timestamp: Double {
Copy link
Contributor

Choose a reason for hiding this comment

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

TimeInterval specifically for Swift.

@velicuvlad
Copy link
Contributor Author

Disabled testCreateArrayIndexWithPath as per LiteCore and filed a ticket for it.

@velicuvlad velicuvlad merged commit eadb932 into master Nov 18, 2024
7 of 9 checks passed
@velicuvlad velicuvlad deleted the enable-vv branch November 18, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pasin pasin pasin 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.
2 participants
@velicuvlad @pasin