Clear Database Test Rule #29

Sloy · 2017-03-14T12:47:53Z

This Rule works similarly to #24, but for SQLite databases.

It fetches all databases in the app, obtains all tables from them, and performs a "delete from" without where.

I decided to do this instead of a DROP because I think it might break the app, not sure though.

This should be tested against different scenarios and sdk versions. I've seen in Stetho that they have different ways of opening databases depending on the sdk version. Wonder why.

TODO:

Check on different sdk versions, and fix any issues
Document on the javadoc and readme
(Won't do) Maybe join with ClearPreferencesRule into one single rule for clearing app data.

Sloy · 2017-03-14T12:51:43Z

🔴 One Issue found: It doesn't work if the database hasn't been created prior to the test (which will be the usual case). We must only try to delete databases if they exist. 🙌

java.lang.RuntimeException: Unable to start activity ComponentInfo{com.schibsted.spain.barista.sample/com.schibsted.spain.barista.sample.DatabaseActivity}: android.database.sqlite.SQLiteException: no such table: User (code 1): , while compiling: SELECT name FROM User
#################################################################
Error Code : 1 (SQLITE_ERROR)
Caused By : SQL(query) error or missing database.
(no such table: User (code 1): , while compiling: SELECT name FROM User)

rocboronat · 2017-03-14T13:42:58Z

Great work! Let's see how it progress!

By the way @Sloy , what do you think about having one Rule for every kind of thing to be remoded, and another one that just joins all of them? I think that having different rules will make them simpler. So, if we want to ease using all of them at the same tame (that will be the most common way to work with Barista), we can just create a Rule to join them. And... well, we can talk about it later, let's focus on this particular Rule :·D

Sloy · 2017-03-14T16:00:17Z

That was my original idea. But I wasn't too sure. I thought about having just one for everything for simplicity, but if you think having both can be useful, let's go with it. I don't have a strong opinion on neither.

rocboronat · 2017-03-14T16:30:14Z

@Sloy yes... the Rule that clears the Database needs 80 lines. The SharedPrefs one, 80 lines more I guess. Let's have them smaller as possible, and create a new Rule to join these Rules.

By the way, at some point of the future, we will need to remove all .jpg and .png pictures of the filesystem (or whatever other extension) so... let's keep that complex rules isolated.

Sloy · 2017-03-15T09:03:01Z

One rule to rule them all!! 💍

rocboronat · 2017-03-15T09:10:06Z

😂

Sloy · 2017-04-03T15:03:31Z

🍏 The issue I commented about clearing databases that don't exist is now fixed.
I changed the way of finding databases and opening them. I had to take into account the shadow database files like those ended in "-journal".

rocboronat · 2017-04-03T15:06:31Z

library/src/main/java/com/schibsted/spain/barista/cleardata/ClearDatabaseRule.java

+ */
+public class ClearDatabaseRule implements TestRule {
+
+  private static final String[] UNINTERESTING_FILENAME_SUFFIXES = new String[] {


rocboronat · 2017-04-03T15:09:54Z

sample/src/main/java/com/schibsted/spain/barista/sample/DatabaseActivity.java

+    writableDatabase.close();
+  }
+
+  private static class OpenHelper extends SQLiteOpenHelper {


We could call it OpenDatabaseHelper. When I seen the dependencies of this Activity, I went crazy: What's this class?

Several horses and monkey died during my madness.

rocboronat · 2017-04-03T15:10:23Z

sample/src/main/java/com/schibsted/spain/barista/sample/DatabaseActivity.java

+  private static class OpenHelper extends SQLiteOpenHelper {
+
+    public OpenHelper(Context context) {
+      super(context, "mydatabase", null, 1);


What means that 1?

rocboronat · 2017-04-03T15:12:23Z

library/src/main/java/com/schibsted/spain/barista/cleardata/ClearDatabaseRule.java

+    return tidiedList;
+  }
+
+  private static String removeSuffix(String str, String[] suffixesToRemove) {


What do you think about calling this method filterUninterestingSuffixes? And I suggest to avoid passing the list of suffixest to remove. It seems that they will be always the same: UNINTERESTING_FILENAME_SUFFIXES.

rocboronat · 2017-04-03T15:13:22Z

library/src/main/java/com/schibsted/spain/barista/cleardata/ClearDatabaseRule.java

+
+  private void clearData() {
+    for (File dbFile : tidyDatabaseList(getDatabaseFiles())) {
+      SQLiteDatabase database = performOpen(dbFile, 0);


What means this 0?

rocboronat · 2017-04-03T15:14:37Z

library/src/main/java/com/schibsted/spain/barista/cleardata/ClearDatabaseRule.java

+   * @param databaseFiles Raw list of database files.
+   * @return Tidied list with shadow databases removed.
+   */
+  private static List<File> tidyDatabaseList(List<File> databaseFiles) {


This method name sounds strange... Don't know. I don't have a suggestion, by the way :·)

rocboronat · 2017-04-03T15:15:41Z

There are some comments but... great work!

Sloy · 2017-04-03T16:36:11Z

Rebased an squashed, much of the code on the rule has been changed @rocboronat.
I will check out your comments on the rest of the code later :)

rocboronat · 2017-04-03T17:07:47Z

Nice, @Sloy ! Check my comments and go on applying the suggestion or merging :·)

Creates Activity and test to check if database is cleared

Sloy · 2017-04-03T18:08:16Z

Travis likes it. I tried a local build with InfoJobs project and everything works as expected.
Any final words @rocboronat?

rocboronat · 2017-04-04T09:50:50Z

Ave, @Sloy , morituri te salutant! Great work there :·) It's merge time! Feel free to release a version if you need it! :·)

Sloy requested a review from rocboronat March 14, 2017 12:47

Sloy force-pushed the feature/clear-database-test-rule branch from 150497f to dab7a5d Compare March 14, 2017 13:16

rocboronat mentioned this pull request Mar 14, 2017

Create a @Rule to clear all databases #25

Closed

rocboronat changed the title ~~[WIP] Clear Database Test Rule~~ Clear Database Test Rule Mar 30, 2017

rocboronat added the wip label Mar 30, 2017

rocboronat reviewed Apr 3, 2017

View reviewed changes

Sloy force-pushed the feature/clear-database-test-rule branch from 64bf82f to 967888a Compare April 3, 2017 16:35

Implemented ClearDatabaseRule

c2c2895

Creates Activity and test to check if database is cleared

Sloy force-pushed the feature/clear-database-test-rule branch from 967888a to c2c2895 Compare April 3, 2017 17:14

Sloy added 6 commits April 3, 2017 19:21

Fixed PMD violation

6e88893

Fixed checkstyle violations

49c70e4

Renamed SqliteOpenHelper for clarity

fac8d52

Extracted database version to a constant for clarity

a72970a

Changed database name to help identify it

4ece685

Added ClearDatabaseRule to the readme

bc98d3e

Sloy removed the wip label Apr 3, 2017

rocboronat approved these changes Apr 4, 2017

View reviewed changes

Sloy merged commit 535108e into master Apr 4, 2017

Sloy deleted the feature/clear-database-test-rule branch April 4, 2017 10:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clear Database Test Rule #29

Clear Database Test Rule #29

Sloy commented Mar 14, 2017 •

edited

Loading

Sloy commented Mar 14, 2017 •

edited

Loading

rocboronat commented Mar 14, 2017

Sloy commented Mar 14, 2017

rocboronat commented Mar 14, 2017 •

edited

Loading

Sloy commented Mar 15, 2017

rocboronat commented Mar 15, 2017

Sloy commented Apr 3, 2017

rocboronat Apr 3, 2017

rocboronat Apr 3, 2017

rocboronat Apr 3, 2017

rocboronat Apr 3, 2017

rocboronat Apr 3, 2017

rocboronat Apr 3, 2017

rocboronat commented Apr 3, 2017

Sloy commented Apr 3, 2017

rocboronat commented Apr 3, 2017

Sloy commented Apr 3, 2017

rocboronat commented Apr 4, 2017

Clear Database Test Rule #29

Clear Database Test Rule #29

Conversation

Sloy commented Mar 14, 2017 • edited Loading

Sloy commented Mar 14, 2017 • edited Loading

rocboronat commented Mar 14, 2017

Sloy commented Mar 14, 2017

rocboronat commented Mar 14, 2017 • edited Loading

Sloy commented Mar 15, 2017

rocboronat commented Mar 15, 2017

Sloy commented Apr 3, 2017

rocboronat Apr 3, 2017

Choose a reason for hiding this comment

rocboronat Apr 3, 2017

Choose a reason for hiding this comment

rocboronat Apr 3, 2017

Choose a reason for hiding this comment

rocboronat Apr 3, 2017

Choose a reason for hiding this comment

rocboronat Apr 3, 2017

Choose a reason for hiding this comment

rocboronat Apr 3, 2017

Choose a reason for hiding this comment

rocboronat commented Apr 3, 2017

Sloy commented Apr 3, 2017

rocboronat commented Apr 3, 2017

Sloy commented Apr 3, 2017

rocboronat commented Apr 4, 2017

Sloy commented Mar 14, 2017 •

edited

Loading

Sloy commented Mar 14, 2017 •

edited

Loading

rocboronat commented Mar 14, 2017 •

edited

Loading