Error when a Watchdog sensor is tried to be added.

Sounds like that's my fault. I wasn't aware that the watchdog sensor is addable.

This was broken by #2543886: Allow database sensor plugins to control which configuration elements are shown. We need what we discussed there, the concept of default configuration so that when you add a new one of a given type, it starts with that set of settings. We will need to discuss this.

;-)

No reason to wonder. It's a bug and it has no test coverage. It definitively needs test coverage by our definition of done.
In cases of a bug, i recommend you to once start with writing the (failing) test first and then start writing code to fix the test.

1. +++ b/src/Form/SensorForm.php
   @@ -237,6 +237,12 @@ class SensorForm extends EntityForm {
   +    if (!empty($plugin->getDefaultConfiguration())) {
   +      $default_config = $plugin->getDefaultConfiguration();
   

   Don't call this twice.

2. +++ b/src/Plugin/monitoring/SensorPlugin/WatchdogAggregatorSensorPlugin.php
   @@ -50,4 +50,15 @@ class WatchdogAggregatorSensorPlugin extends DatabaseAggregatorSensorPlugin impl
   +       'table' => 'watchdog',
   +       'time_interval_field' => 'timestamp',
   

   Wrong indentation.

3. +++ b/src/SensorPlugin/SensorPluginInterface.php
   @@ -30,6 +30,14 @@ interface SensorPluginInterface extends PluginInspectionInterface, PluginFormInt
   +  public function getDefaultConfiguration();
   

   Mind the core ConfigurablePluginInterface. Should we reuse it here?

3. No, we can't. That assumes configuration is an array, but we use the sensor config entity directly. Would be weird to mix that.

+++ b/src/Tests/MonitoringUITest.php
@@ -177,6 +177,16 @@ class MonitoringUITest extends MonitoringTestBase {
+
+    $this->drupalPostForm(NULL, array(), t('Save'));
+    $this->assertText(SafeMarkup::format('Sensor @label saved.', array('@label' => 'Watchdog Sensor')));

Empty line here, and the SafeMarkup::format is no needed here, guess that t() should be enough. have a look at

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Util...

1. +++ b/src/Form/SensorForm.php
   @@ -237,6 +237,11 @@ class SensorForm extends EntityForm {
   +    if ($default_config = $plugin->getDefaultConfiguration()) {
   +      $form_state->setValue(['settings', 'table'], $default_config['table']);
   +      $form_state->setValue(['settings', 'aggregation', 'time_interval_field'], $default_config['time_interval_field']);
   
   +++ b/src/Plugin/monitoring/SensorPlugin/WatchdogAggregatorSensorPlugin.php
   @@ -50,4 +50,15 @@ class WatchdogAggregatorSensorPlugin extends DatabaseAggregatorSensorPlugin impl
   +    $default_config = [
   +      'table' => 'watchdog',
   +      'time_interval_field' => 'timestamp',
   

   Use the same structure in default configuration as in the settings. Just set the entity settings and it should work.

2. +++ b/src/Tests/MonitoringUITest.php
   @@ -177,6 +177,15 @@ class MonitoringUITest extends MonitoringTestBase {
   +    // Test the creation of a Watchdog sensor.
   

   Plz mention that we are testing default configuration here.

Much better. Almost. ;-)

+++ b/src/Form/SensorForm.php
@@ -237,6 +237,10 @@ class SensorForm extends EntityForm {
+    if ($default_config = $plugin->getDefaultConfiguration()) {
+      $form_state->setValue('settings', $default_config);
+    }

This should not be in validateForm().
Default config is about default values, before the form is displayed. So the form should represent the default values.

Set the entity settings on the entity in form() around:

  public function form(array $form, FormStateInterface $form_state) {
    $form = parent::form($form, $form_state);

    $sensor_config = $this->entity;

Exactly. Now one last thing more to make test coverage happy.

+++ b/src/Tests/MonitoringUITest.php
@@ -177,6 +177,15 @@ class MonitoringUITest extends MonitoringTestBase {
+    $this->drupalGet('admin/config/system/monitoring/sensors/add');

Here you should check that the default values are filled out in the form. :-)

Sorry, but miro is wrong :)

Your code *overwrites* existing settings every time the form is displayed.

As discussed, the correct place to set this is submitSelectPlugin(), which is where the plugin *changes*. Then we need to reset the settings back to the defaults of that plugin type. Always, even if they are empty.

Also, you can't test this in the UI since those settings aren't visible. You need to actually run the sensor to test it.

The code seems fine, still waiting for the test to pass.

Much nice now. Monitoring HEAD is broken, so we can wait forever until this is green... ;-)

Awesome, working default configuration with nice test coverage!
Fails unrelated, the UI test passes.