Follow-up to #2581697: Test are failing because of recent core changes.

Problem/Motivation

Now the sensor depends on automated_cron module so we should add the dependencies on the Sensor

Proposed resolution

Add dependencies using function calculateDependencies().

Comments

giancarlosotelo created an issue. See original summary.

giancarlosotelo’s picture

Status: Active » Needs review
StatusFileSize
new1.1 KB

Added the function, wondering if it is enough.

berdir’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

As a first step, yes.

Additionally, it would be good to add a dependency on the actual config in case it is a config entity. So if we check for a specific view, we should also directly depend on that config.

I thought about doing that in a follow-up, but that's actually quite easy to identify. Load the config, if it exists and has a dependency key on the first level, we can assume it is a config entity, then add that to the dependencies too.

We should also have some test coverage for this, we can probably just extend our existing tests.

giancarlosotelo’s picture

Status: Needs work » Needs review
StatusFileSize
new2.31 KB
new1.74 KB

Ok, I think comments above are addressed.

juanse254’s picture

Tested locally seems to work.

juanse254’s picture

Status: Needs review » Reviewed & tested by the community
miro_dietiker’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/src/Tests/MonitoringUITest.php
@@ -266,6 +266,17 @@ class MonitoringUITest extends MonitoringTestBase {
+    $this->assertTrue($dependencies['config']);

I think that should be some string that refers to the specific config object... Not just plain TRUE... We should specifically test this.

giancarlosotelo’s picture

Status: Needs work » Needs review
StatusFileSize
new2.34 KB
new624 bytes

Changed on test.

miro_dietiker’s picture

Status: Needs review » Fixed

Much nice! Thx, committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.