This repository was archived by the owner on Feb 22, 2023. It is now read-only.
[flutter_plugin_tool] Fix CHANGELOG validation failure summary #4266
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The error summary for a CHANGELOG validation failure was written when the only thing being checked was that the versions matched, but now there are other ways to fail as well (i.e., leaving NEXT). This fixes the summary message to be more generic so that it doesn't mislead people who hit validation failures. While adding the test for this message, I discovered that almost all of the tests were actually talking to pub.dev, causing their behavior to in some cases depend on whether a package with that name happened to have been published, and if so what its version was. In order to make the tests hermetic and predictable, this fixes that by making all tests use a mock HTTP client.
11 tasks
cyanglaz
approved these changes
Aug 25, 2021
| if (!(await _validateChangelogVersion(package, | ||
| pubspec: pubspec, pubspecVersionChanged: versionChanged))) { | ||
| errors.add('pubspec.yaml and CHANGELOG.md have different versions'); | ||
| errors.add('CHANGELOG.md failed validation.'); |
Contributor
There was a problem hiding this comment.
Do we have a place to explain what are the failing scenarios? If not, maybe we should list them here so it's easier for debugging. (Especially when none-team-member contributor hits this error)
Contributor
Author
There was a problem hiding this comment.
The specific error is output inline, this is just the line that goes in the final summary. The summary block always says to see the full log for details. See https://cirrus-ci.com/task/4541043946291200 for an example (that led to this PR).
| fileSystem = MemoryFileSystem(); | ||
| mockPlatform = MockPlatform(); | ||
| packagesDir = createPackagesDirectory(fileSystem: fileSystem); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
I added this on purpose; the next section is a big block of setup for the mock GitDir and having it separated out slightly makes it easier to see the high-level flow vs it being a wall of text.
(I'll likely replace the git setup here with something based on processRunner like I did with the new publish tests, which would help, but didn't want to get into that in this PR.)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 26, 2021
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 26, 2021
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 26, 2021
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 26, 2021
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 26, 2021
fotiDim
pushed a commit
to fotiDim/plugins
that referenced
this pull request
Sep 13, 2021
…er#4266) The error summary for a CHANGELOG validation failure was written when the only thing being checked was that the versions matched, but now there are other ways to fail as well (i.e., leaving NEXT). This fixes the summary message to be more generic so that it doesn't mislead people who hit validation failures. While adding the test for this message, I discovered that almost all of the tests were actually talking to pub.dev, causing their behavior to in some cases depend on whether a package with that name happened to have been published, and if so what its version was. In order to make the tests hermetic and predictable, this fixes that by making all tests use a mock HTTP client.
amantoux
pushed a commit
to amantoux/plugins
that referenced
this pull request
Sep 27, 2021
…er#4266) The error summary for a CHANGELOG validation failure was written when the only thing being checked was that the versions matched, but now there are other ways to fail as well (i.e., leaving NEXT). This fixes the summary message to be more generic so that it doesn't mislead people who hit validation failures. While adding the test for this message, I discovered that almost all of the tests were actually talking to pub.dev, causing their behavior to in some cases depend on whether a package with that name happened to have been published, and if so what its version was. In order to make the tests hermetic and predictable, this fixes that by making all tests use a mock HTTP client.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The error summary for a CHANGELOG validation failure was written when
the only thing being checked was that the versions matched, but now
there are other ways to fail as well (i.e., leaving NEXT). This fixes
the summary message to be more generic so that it doesn't mislead people
who hit validation failures.
While adding the test for this message, I discovered that almost all of
the tests were actually talking to pub.dev, causing their behavior to in
some cases depend on whether a package with that name happened to have
been published, and if so what its version was. In order to make the
tests hermetic and predictable, this fixes that by making all tests use
a mock HTTP client.
Pre-launch Checklist
dart format.)[shared_preferences]///).