Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FED-734 CI cleanup: remove failing stable config, add 2.18 config #805

Merged
merged 16 commits into from
Feb 8, 2023

Conversation

greglittlefield-wf
Copy link
Contributor

@greglittlefield-wf greglittlefield-wf commented Feb 7, 2023

Motivation

  • The GitHub Actions "build" job, which is our main set of CI checks, hasn't only been running on 2.13.4 for a long time, and nothing newer. I'd really like to include a build for 2.18
  • The "validate_analyzer" job is failing on Dart stable due to a Dart SDK bug that we're unable to work around at this time, so we want to disable it.

Changes

  • Add 2.18.7 to the build matrix, since it's the latest Dart SDK version we can build with
    • Fix test failures in newer SDKs
    • Ignore part_of_different_library errors
    • Upgrade mockito since it older versions were causing build failures (merge in Upgrade to mockito v5 #789) - thanks @evanweible-wf!
    • Fix usages of the dartanalyzer command removed in newer SDKs
    • Fix VM test command to not use build_runner
    • Update built_redux/mockito generated file checks to only run in 2.18.7
  • Remove validate_analyzer config, since it isn't needed once Raise analyzer minimum & unpin meta #804 constrains our analyzer dependency to a single major version
    • We can reinstate this if we ever need to support multiple versions again in the future

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

  • Tests were updated and provide good coverage of the changeset and other affected code
  • Manual testing was performed if needed

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

  • A Frontend Frameworks Design member has reviewed these changes
  • There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
  • For release PRs - Version metadata in Rosie comment is correct

@aviary-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

@@ -7,7 +7,7 @@ environment:
dependencies:
color: any
memoize: ^2.0.0
meta: ^1.6.0 # Workaround to avoid https://github.com/dart-lang/sdk/issues/46142
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments were outdated, so I cleaned them up

@@ -1063,19 +1063,25 @@ main() {
expect(result2, same(result1), reason: 'should have returned the same object');
}, tags: 'no-ddc');

test('unless the runtime is the DDC', () {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, before we were testing that caching wasn't happening in DDC, which wasn't really the behavior we wanted ensure. I replaced it with a test that validates that it doesn't break, which is really the behavior we're after, at least for older SDKs.

Failures look like:
    Failed to load "test/over_react_component_declaration_test.dart": Timed out waiting for Chrome to connect
@greglittlefield-wf greglittlefield-wf marked this pull request as ready for review February 8, 2023 01:24
@greglittlefield-wf greglittlefield-wf changed the title CI cleanup: remove failing stable config, add 2.18 config CI cleanup: remove failing stable config, add 2.18 config, upgrade mockito Feb 8, 2023
@rmconsole6-wk rmconsole6-wk changed the title CI cleanup: remove failing stable config, add 2.18 config, upgrade mockito FED-734 CI cleanup: remove failing stable config, add 2.18 config Feb 8, 2023
Copy link
Member

@robbecker-wf robbecker-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the detailed comments on why things were done.

@robbecker-wf
Copy link
Member

QA+1 CI Passes on 2.13 and 2.18
tests updated and pass with newer mockito

@greglittlefield-wf
Copy link
Contributor Author

@Workiva/release-management-p

Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 from RM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants