From 6876b5b22a6b0be343a7b46f148d301abaeaa75a Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Fri, 20 Dec 2024 23:53:32 +0100 Subject: [PATCH 1/3] More robust location definitions Say you had this Oaken seed file: ```ruby users.create :kasper, name: "Kasper" ``` We want `users.method(:kasper)` to point back to that file and line to help debugging. However, we had a hardcoded db/seeds in our detection, which meant that create/upsert/label in tests would fail on a NoMehodError because our `find` wouldn't return anything. In tests, labels make less sense in general, so I'm thinking we should allow those. I'm also thinking I should try to more sharply define the preparation phase versus the running phase. The preparation phase: loading and executing db/seeds.rb, setup defaults, helpers and common records The running phase: when running tests, you shouldn't mutate anything really, and labels shouldn't suddenly spring up, generally things should be deterministic. `Oaken.preparing?`/`Oaken.running?` may be on the table too. Note: this does get complicated if people are using `seed "cases/pagination"` with one-off seeds. Hm. Anyway, I'm detecting the path via a quirk on the label + our loading so we don't need to use `lookup_paths` at all. I'm not sure I'm liking any of this. --- lib/oaken.rb | 6 ++++++ lib/oaken/stored/active_record.rb | 12 +++++++----- test/dummy/test/models/oaken_test.rb | 6 ++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/oaken.rb b/lib/oaken.rb index 8d79d86..504bbaf 100644 --- a/lib/oaken.rb +++ b/lib/oaken.rb @@ -39,6 +39,12 @@ def load_onto(seeds) = @entries.each do |path| seeds.class_eval path.read, path.to_s end end + + def self.definition_location + # Trickery abounds! Due to Ruby's `caller_locations` + our `load_onto`'s `class_eval` above + # we can use this format to detect the location in the seed file where the call came from. + caller_locations(2, 8).find { _1.label.match? /block .*?in load_onto/ } + end end def self.prepare(&block) = Seeds.instance_eval(&block) diff --git a/lib/oaken/stored/active_record.rb b/lib/oaken/stored/active_record.rb index 279f3e9..26df669 100644 --- a/lib/oaken/stored/active_record.rb +++ b/lib/oaken/stored/active_record.rb @@ -49,11 +49,13 @@ def upsert(label = nil, unique_by: nil, **attributes) # # Note: `users.method(:someone).source_location` also points back to the file and line of the `label` call. def label(**labels) - # TODO: Fix hardcoding of db/seeds instead of using Oaken.lookup_paths - location = caller_locations(1, 6).find { _1.path.match? /db\/seeds\// } + labels.transform_values(&:id).each { _label _1, _2 } + end + + private def _label(name, id) + raise ArgumentError, "you can only define labelled records outside of tests" \ + unless location = Oaken::Loader.definition_location - labels.each do |label, record| - class_eval "def #{label} = find(#{record.id.inspect})", location.path, location.lineno - end + class_eval "def #{name} = find(#{id.inspect})", location.path, location.lineno end end diff --git a/test/dummy/test/models/oaken_test.rb b/test/dummy/test/models/oaken_test.rb index ee00530..035fc43 100644 --- a/test/dummy/test/models/oaken_test.rb +++ b/test/dummy/test/models/oaken_test.rb @@ -76,6 +76,12 @@ def self.column_names = [] assert_match "db/seeds/test/data/users.rb", users.method(:test_user).source_location.first end + test "can't use labels within tests" do + assert_raise ArgumentError do + users.label kasper_2: users.kasper + end + end + test "updating fixture" do users.kasper.update name: "Kasper2" assert_equal "Kasper2", users.kasper.name From c81d9a542dd4ea5ab38e5a9af6e717be8745f7ae Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sat, 21 Dec 2024 00:07:43 +0100 Subject: [PATCH 2/3] Ruby 3.4 changed the format, hm --- lib/oaken.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/oaken.rb b/lib/oaken.rb index 504bbaf..f38bccc 100644 --- a/lib/oaken.rb +++ b/lib/oaken.rb @@ -43,7 +43,7 @@ def load_onto(seeds) = @entries.each do |path| def self.definition_location # Trickery abounds! Due to Ruby's `caller_locations` + our `load_onto`'s `class_eval` above # we can use this format to detect the location in the seed file where the call came from. - caller_locations(2, 8).find { _1.label.match? /block .*?in load_onto/ } + caller_locations(2, 8).find { _1.label.match? /block .*?load_onto/ } end end From 2f7b4d842cde0426e25aa3dc370da33c83a9e267 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Thu, 9 Jan 2025 02:26:56 +0100 Subject: [PATCH 3/3] Don't need to transform_values to avoid capturing the record reference I don't think --- lib/oaken/stored/active_record.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/oaken/stored/active_record.rb b/lib/oaken/stored/active_record.rb index 26df669..b618ba1 100644 --- a/lib/oaken/stored/active_record.rb +++ b/lib/oaken/stored/active_record.rb @@ -49,7 +49,7 @@ def upsert(label = nil, unique_by: nil, **attributes) # # Note: `users.method(:someone).source_location` also points back to the file and line of the `label` call. def label(**labels) - labels.transform_values(&:id).each { _label _1, _2 } + labels.each { |label, record| _label label, record.id } end private def _label(name, id)