Skip to content

Unit tests for PrimaryKey field type change#3076

Merged
stk1m1 merged 7 commits intoreleasesfrom
stk/test/pktypechange
Jul 5, 2016
Merged

Unit tests for PrimaryKey field type change#3076
stk1m1 merged 7 commits intoreleasesfrom
stk/test/pktypechange

Conversation

@stk1m1
Copy link
Copy Markdown
Contributor

@stk1m1 stk1m1 commented Jun 27, 2016

Unit tests for checking proper migration of PrimaryKey field type change from String to Integer/int. These cases were surfaced in issue #3068.

@stk1m1 stk1m1 self-assigned this Jun 27, 2016
CHANGELOG.md Outdated

### Enhancements

* Unit tests for checking proper migration of primary key field type change from `String` to `Integer`/`int` (#3068).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We normally don't put internal changes in the changelog

@stk1m1
Copy link
Copy Markdown
Contributor Author

stk1m1 commented Jun 27, 2016

@realm/java please review.


public static String FIELD_PRIMARY = "fieldIntPrimary";

private Byte fieldFirst;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The test class can be much shorter by removing the getters/setters and just using the public field.

@beeender
Copy link
Copy Markdown
Contributor

beeender commented Jul 5, 2016

👍 with a minor comment


private void createObjectsWithOldPrimaryKey(final String className, final boolean insertNullValue) {
DynamicRealm realm = DynamicRealm.getInstance(configFactory.createConfigurationBuilder().build());
realm.executeTransaction(new DynamicRealm.Transaction() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's use try-finally

@zaki50
Copy link
Copy Markdown
Contributor

zaki50 commented Jul 5, 2016

I don't think we need to update asset_file.realm since I recently added a module class for that Realm file.

Please merge latest releases branch and check again.

@stk1m1
Copy link
Copy Markdown
Contributor Author

stk1m1 commented Jul 5, 2016

retest this please

@zaki50
Copy link
Copy Markdown
Contributor

zaki50 commented Jul 5, 2016

👍

1 similar comment
@kneth
Copy link
Copy Markdown
Contributor

kneth commented Jul 5, 2016

👍

@stk1m1 stk1m1 merged commit 1d83740 into releases Jul 5, 2016
@stk1m1 stk1m1 removed the S:Review label Jul 5, 2016
@stk1m1 stk1m1 deleted the stk/test/pktypechange branch July 5, 2016 11:38
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants