test: add missing tests for Entity+Edge model migration#513
Conversation
Restore and strengthen test coverage that was lost during the Asset → Entity+Edge migration. Adds new test files for previously untested components and extends existing test files with edge-case coverage. New test files: - processors/enrich: Init validation, Process with nil/existing props, edge passthrough - processors/labels: Init validation, Process with nil/existing labels, merge, edge passthrough - sinks/console: Init, Sink single/multiple/empty/with-edges, Close - sinks/kafka: Init config validation (missing brokers, topic, etc.) Strengthened tests: - models/record: multiple edges, nil entity - models/util: nil/empty props, nested props sanitization, JSON with/without edges - processors/script: modify entity name, nil properties, edge preservation - sinks/compass: empty batch handling - sinks/file: empty batch, records with edges (ndjson + yaml)
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request expands test coverage across the models, processors, and sinks packages. It adds unit tests for core model operations including 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/sinks/file/file_test.go (1)
84-140: Consider writing outputs tot.TempDir()for better test isolation.Using checked-in fixture paths as write targets can introduce shared mutable state between tests.
♻️ Suggested test-isolation refactor
+import "path/filepath" ... - config := map[string]any{ - "path": "./test-dir/edges.ndjson", - "format": "ndjson", - } + tmpDir := t.TempDir() + config := map[string]any{ + "path": filepath.Join(tmpDir, "edges.ndjson"), + "format": "ndjson", + }Apply similarly to the yaml and empty-batch subtests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/sinks/file/file_test.go` around lines 84 - 140, Tests currently write to checked-in fixture paths which creates shared mutable state; update each subtest ("should sink empty batch without error", "should sink records with edges in ndjson format", "should sink records with edges in yaml format") to use t.TempDir() and set the config["path"] to a file inside that temp dir (e.g., filepath.Join(tmpDir, "edges.ndjson" or "edges.yaml") ), keeping the same use of f.New, fileSink.Init and fileSink.Sink calls; this ensures isolated writable paths per test and removes reliance on checked-in directories.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/sinks/file/file_test.go`:
- Around line 84-140: Tests currently write to checked-in fixture paths which
creates shared mutable state; update each subtest ("should sink empty batch
without error", "should sink records with edges in ndjson format", "should sink
records with edges in yaml format") to use t.TempDir() and set the
config["path"] to a file inside that temp dir (e.g., filepath.Join(tmpDir,
"edges.ndjson" or "edges.yaml") ), keeping the same use of f.New, fileSink.Init
and fileSink.Sink calls; this ensures isolated writable paths per test and
removes reliance on checked-in directories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4437f22e-0308-4e27-94f6-3b6a82a4ea79
📒 Files selected for processing (11)
models/record_test.gomodels/util_test.goplugins/processors/enrich/processor_test.goplugins/processors/labels/labels_test.goplugins/processors/script/tengo_script_test.goplugins/sinks/compass/sink_test.goplugins/sinks/console/sink_test.goplugins/sinks/file/file_test.goplugins/sinks/file/test-dir/edges.ndjsonplugins/sinks/file/test-dir/edges.yamlplugins/sinks/kafka/sink_test.go
Replace outdated Asset terminology with the current Record/Entity/Edge model that was introduced in the migration.
Update PROTON_COMMIT to the commit that added the new Entity+Edge protos and removed the legacy assets. Update the --path flag from the deleted raystack/assets/v1beta2 to raystack/meteor/v1beta1.
Summary
New test files
processors/enrichprocessors/labelssinks/consolesinks/kafkaStrengthened tests
models/recordmodels/utilprocessors/scriptsinks/compasssinks/fileTest plan
go test -tags=plugins ./...