yogthos / migratus

MIGRATE ALL THE THINGS!
641 stars 95 forks source link

Rollback using applied date #233

Closed S4G4R closed 1 year ago

S4G4R commented 1 year ago

Fixes #199

yogthos commented 1 year ago

Could you also take a look at updating the tests?

S4G4R commented 1 year ago

Could you also take a look at updating the tests?

My tests succeeded (With lein test). Any pointers as to what I need to do?

yogthos commented 1 year ago

I'm seeing this error that traces to (sort-by :applied #(.compareTo %2 %1)). Looks like the fix might be to do (sort-by :applied #(if (and %1 %2) (.compareTo %2 %1) -1)) instead?

lein test :only migratus.test.database/test-migrate                                                                                         

ERROR in (test-migrate) (Reflector.java:97)                                                                                                 
Uncaught exception, not in assertion.                                                                                                              
expected: nil                                                                                                                                      
  actual: java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "target" is null
 at clojure.lang.Reflector.invokeInstanceMethod (Reflector.java:97)
    migratus.database$completed_ids_STAR_$fn__3755.invoke (database.clj:170)
    clojure.lang.AFunction.compare (AFunction.java:51)
    clojure.core$sort_by$fn__5978.invoke (core.clj:3131)
    clojure.lang.AFunction.compare (AFunction.java:51)
    java.util.TimSort.countRunAndMakeAscending (TimSort.java:355)
    java.util.TimSort.sort (TimSort.java:220)
    java.util.Arrays.sort (Arrays.java:1233)
    clojure.core$sort.invokeStatic (core.clj:3115)
    clojure.core$sort_by.invokeStatic (core.clj:3119)
    clojure.core$sort_by.invoke (core.clj:3119)
    migratus.database$completed_ids_STAR_.invokeStatic (database.clj:170)
    migratus.database$completed_ids_STAR_.invoke (database.clj:165)
    migratus.database.Database.completed_ids (database.clj:292)
    migratus.core$run_down.invokeStatic (core.clj:129)
    migratus.core$run_down.invoke (core.clj:128)
    clojure.core$partial$fn__5839.invoke (core.clj:2625)
    migratus.core$run.invokeStatic (core.clj:54)
    migratus.core$run.invoke (core.clj:50)
    migratus.core$down.invokeStatic (core.clj:144)
    migratus.core$down.doInvoke (core.clj:140)
    clojure.lang.RestFn.invoke (RestFn.java:423)
    migratus.test.database$fn__4404.invokeStatic (database.clj:153)
    migratus.test.database/fn (database.clj:143)
    clojure.test$test_var$fn__9737.invoke (test.clj:717)
    clojure.test$test_var.invokeStatic (test.clj:717)
    clojure.test$test_var.invoke (test.clj:708)
    clojure.test$test_vars$fn__9763$fn__9768.invoke (test.clj:735)
    migratus.test.migration.sql$setup_test_db.invokeStatic (sql.clj:37)
    migratus.test.migration.sql$setup_test_db.invoke (sql.clj:35)
    clojure.test$compose_fixtures$fn__9731$fn__9732.invoke (test.clj:694)
S4G4R commented 1 year ago

I'm seeing this error that traces to (sort-by :applied #(.compareTo %2 %1)). Looks like the fix might be to do (sort-by :applied #(if (and %1 %2) (.compareTo %2 %1) -1)) instead?

lein test :only migratus.test.database/test-migrate                                                                                         

ERROR in (test-migrate) (Reflector.java:97)                                                                                                 
Uncaught exception, not in assertion.                                                                                                              
expected: nil                                                                                                                                      
  actual: java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "target" is null
 at clojure.lang.Reflector.invokeInstanceMethod (Reflector.java:97)
    migratus.database$completed_ids_STAR_$fn__3755.invoke (database.clj:170)
    clojure.lang.AFunction.compare (AFunction.java:51)
    clojure.core$sort_by$fn__5978.invoke (core.clj:3131)
    clojure.lang.AFunction.compare (AFunction.java:51)
    java.util.TimSort.countRunAndMakeAscending (TimSort.java:355)
    java.util.TimSort.sort (TimSort.java:220)
    java.util.Arrays.sort (Arrays.java:1233)
    clojure.core$sort.invokeStatic (core.clj:3115)
    clojure.core$sort_by.invokeStatic (core.clj:3119)
    clojure.core$sort_by.invoke (core.clj:3119)
    migratus.database$completed_ids_STAR_.invokeStatic (database.clj:170)
    migratus.database$completed_ids_STAR_.invoke (database.clj:165)
    migratus.database.Database.completed_ids (database.clj:292)
    migratus.core$run_down.invokeStatic (core.clj:129)
    migratus.core$run_down.invoke (core.clj:128)
    clojure.core$partial$fn__5839.invoke (core.clj:2625)
    migratus.core$run.invokeStatic (core.clj:54)
    migratus.core$run.invoke (core.clj:50)
    migratus.core$down.invokeStatic (core.clj:144)
    migratus.core$down.doInvoke (core.clj:140)
    clojure.lang.RestFn.invoke (RestFn.java:423)
    migratus.test.database$fn__4404.invokeStatic (database.clj:153)
    migratus.test.database/fn (database.clj:143)
    clojure.test$test_var$fn__9737.invoke (test.clj:717)
    clojure.test$test_var.invokeStatic (test.clj:717)
    clojure.test$test_var.invoke (test.clj:708)
    clojure.test$test_vars$fn__9763$fn__9768.invoke (test.clj:735)
    migratus.test.migration.sql$setup_test_db.invokeStatic (sql.clj:37)
    migratus.test.migration.sql$setup_test_db.invoke (sql.clj:35)
    clojure.test$compose_fixtures$fn__9731$fn__9732.invoke (test.clj:694)

Didn't catch this somehow. I'll submit a fix in a bit.