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

Initial commit of override tooling #4158

Merged
4 commits merged into from
Feb 22, 2020
Merged

Conversation

NickGerleman
Copy link
Collaborator

@NickGerleman NickGerleman commented Feb 21, 2020

Add a foundation for new override tooling described in #4104. This includes:

  • Build scripts, lint scripts, config files, etc
  • Logic for parsing and checking validity of an override manifest
  • Unit tests for override manifest logic
  • Abstractions to allow fetching React Native files of arbtrary versions

A lot of this is foundational. The override logic has been well-tested,
and the Git logic has been manually tested, but we don't have much
end-to-end set up yet.

Microsoft Reviewers: Open in CodeFlow

Add a foundation for new override tooling described in microsoft#4104. This includes:

- Build scripts, lint scripts, config files, etc
- Logic for parsing and checking validity of an override manifest
- Unit tests for override manifest logic
- Abstractions to allow fetching React Native files of arbtrary versions

A lot of this is foundational. The override logic has been well-tested,
and the Git logic has been manually tested, but we don't have much
end-to-end set up yet.
@NickGerleman NickGerleman requested a review from a team as a code owner February 21, 2020 00:33
@NickGerleman
Copy link
Collaborator Author

@acoates-ms do we have a nice way to hook in jest tests for a package into CI? Having tests locally only for just this isn't too bad, but if it's easy, I would rather add JS tests to there as well.

@acoates-ms
Copy link
Contributor

We should introduce a new top level command for "test", which would run UTs.
Should be easy to add a new command to package.json that runs lerna to run the test command on all the packages.

Then pick one of the CI slices to trigger the test command. (I generally look at which one is currently quickest). -- you'll want to make sure its one of the ones that has already done yarn build.

.. Then only other tricky part is that it looks like E2E test package is already using the test script for something. ... So then I guess either pick a different name than test either for the new task, or rename the existing E2E one, which probably also means modifying the CI loop.

@NickGerleman NickGerleman requested a review from a team February 21, 2020 01:27
Copy link

@NikoAri NikoAri left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Member

@vmoroz vmoroz left a comment

Choose a reason for hiding this comment

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

:shipit:

@NickGerleman
Copy link
Collaborator Author

We should introduce a new top level command for "test", which would run UTs.
Should be easy to add a new command to package.json that runs lerna to run the test command on all the packages.

Then pick one of the CI slices to trigger the test command. (I generally look at which one is currently quickest). -- you'll want to make sure its one of the ones that has already done yarn build.

.. Then only other tricky part is that it looks like E2E test package is already using the test script for something. ... So then I guess either pick a different name than test either for the new task, or rename the existing E2E one, which probably also means modifying the CI loop.

Looks like using the just-task template actually puts unit tests as part of the build. This seems reasonable, since on my machine it takes 5s to run tests without the jest cache. That should get us CI coverage without any extra work.

@NickGerleman NickGerleman added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label Feb 21, 2020
@ghost
Copy link

ghost commented Feb 21, 2020

Hello @NickGerleman!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

We hardcode an old version of WebderiverIO beacuse of microsoft#3019. These seem to have loose dependency requirements, because the change to deuplicate packages broke this (see webdriverio/webdriverio#4104). Hardcode resolutions in E2ETest for existing versions of wdio packages in the meantime.
@ghost ghost merged commit 638c698 into microsoft:master Feb 22, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants