Silences warning logs and adds additional earlier skip check if no detected BCD updates

[ChrisC]

This is an initial pass at implementing @foolip's suggestion in #899. This:

1. Sets the reason logger to "quiet" by default.
2. Adds a new skip check & utility helper function to check for any differences in the support matrix and the existing BCD support statements. If there's no detected differences, we exit early, otherwise precede with the rest of the existing update logic. We also run this "contradiction" check once more at the final skip scenario (in the case that we still have contradictions that the updater was unable to resolve). If we do find ourselves at this step, we "audibly" log a warning.

Some General Notes
Before this change, many calls to update from running the unit tests reached the final skip check and emitted a "skipped for no known reason" message, which was a bit of a false alarm. For now, I opted to run some simple diffs limited to the version_added field. But even with these simple diff checks, we now no longer reach that final "unknown reason" skip check on a unit test run.

These initial logs should now be more useful for spotlighting any update issues for specific BCD entries. Here's a snippet of log output from a recent manual run:

warn: api.EXT_texture_compression_rgtc skipped for firefox with unresolved differences between support matrix and BCD data. Possible intervention required.
info: Updating api/Element.json
info: Updating api/FontFaceSet.json
info: Updating api/HTMLAnchorElement.json
warn: api.HTMLCanvasElement.getContext.webgl2_context.options_powerPreference_parameter skipped for firefox with unresolved differences between support matrix and BCD data. Possible intervention required.
warn: api.HTMLElement.autofocus skipped for firefox with unresolved differences between support matrix and BCD data. Possible intervention required.

Further work on generating more useful logs will definitely help here. The unit tests should give an accurate summary of all the types of "contradictions" that we're currently checking for.

---

Suggested Future Improvements

• A lot of these new diff checks are duplicative of checks that happen later when we try to infer a support statement from the support matrix, or when we go through the various persist... methods. We may be able to "provide" the results of these checks to the UpdateState for later use by other methods. For now, we're just checking for the existence of any contradictions and then checking again later when it's time to update.
• I was able to defer the building of the inferredStatements, but we maybe able to defer or skip other unnecessary steps if we make better use of this diff data from the get-go.

cc @mzgoddard

[ChrisC]

Here's a before-and-after illustration of the changes to the data flow in this PR (just excerpted to the start of the browserMap loop).
Before

After

[foolip]

Thanks for putting this together @ChrisC! The overall structure looks good to me, and I'll do a detailed review next week.

[foolip]

A few comments. Most importantly, I think we should use defaultStatements.

[foolip]

I think we should pass in defaultStatements here, so that we can detect contradictions when there are multiple version ranges involved. We aren't able to update all such cases yet, but that should then result in a warning.

[foolip]

I think this would be the signature if we pass in defaultStatements:

Suggested change

	simpleStatement?: SimpleSupportStatement,
	statements: SimpleSupportStatement[],

It could be an empty array, but not null. An example of what an array might be:

"support": {
  "chrome": [
    {
      "version_added": "30"
    },
    {
      "version_added": "10",
      "version_removed": "20"
    }
  ]
}

If we may have tests result showing support in Chrome 110, and the lack of support in Chrome 29, that is consistent with BCD. But if we have test results showing support in Chrome 9, that is a contradiction.

In my following comments, I'll assume that we iterate over versionMap, skip any nulls, and inside that loop iterate over statements, which is usually zero or one item, but possibly more.

[foolip]

@ChrisC do you plan to address this? If so I'll hold off with review of the recent changes.

[ChrisC]

@ChrisC do you plan to address this? If so I'll hold off with review of the recent changes.

@foolip I did try to address this and was hoping that with some smart sorting of the statements and versions, this would be simple to implement. Unfortunately, that approach did not work reliably.

Instead, I'm going to add another provide... method after we get the defaultStatements to infer supported and unsupported ranges to check each version against. That will be the most robust solution for checking version support against multiple default statements.

I would prefer to do that in a separate PR if possible (since I think it will be quite a bit more code to add), but if you have a strong preference for keeping it in this PR, I can continue with that work here.

[foolip]

That sounds great @ChrisC, and fine to do it in a separate PR since it's not as simple as I thought initially.

[foolip]

Here's untested code of how I thought testing against an array of statements (defaultStatements) would probably work:

// Test a single version (string) against a BCD statements array.
// Note that order of statements does not matter.
function isSupported(version, statements) {
  for (const { version_added, version_removed } of statements) {
    if (version_added && compare(version, version_added, '>=') {
      if (version_removed && compare(version, version_removed, '>=') {
        continue;
      }
      return true;
    }
  }
  return false;
}

function hasSupportUpdates(versionMap, statements) {
  for (const [version, hasSupport] of versionMap.entries()) {
    if (hasSupport === null) {
      continue;
    }
    if (hasSupport !=== isSupported(version, statements) {
      return true;
    }
  }
}

In other words, for each true or false test result for a specific version (from collector results), test it against the support statements, once range at a time.

[ChrisC]

This is more or less what I tried. However, take the array of statements you provided as an example ☝🏽.

"support": {
  "chrome": [
    {
      "version_added": "30"
    },
    {
      "version_added": "10",
      "version_removed": "20"
    }
  ]
}

Let's say you're testing version 10 with a a test result of hasSupport === true against this set of support statements. This should not show any contradictions. However, when tested for support against each statement individually, it would return false when tested against the the statement of version_added: 30 and then send out a false positive signal for the need to update the statements. (Note that it would return true against the other statement.)

I suppose you could collect all the comparisons when iterating through the statements, and assume that as long as there's no inconsistency between the individual statement comparisons, there there's no need to update them. But that seemed a bit brittle and non-specific to me. See another approach where we test against ranges more specifically in #967.

[foolip]

I'll take a look at your new PR, but for now just want to say that I don't think we should test a version ("10" in your example) against a part of a support statement, only the full array as one test.

If there is a contradiction we do need to figure out how which part of a support statement to update, or if we should add/remove a part, but I consider that a problem of handling more cases correctly, not identifying when an update is needed and logging.