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

[Circle CI] Add test generated project job on Windows #412

Merged
merged 35 commits into from
Mar 3, 2022

Conversation

adamraya
Copy link
Collaborator

@adamraya adamraya commented Feb 17, 2022

GUS: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00000mBHOTYA4/view

Description

The PR adds a new Circle CI job to generate a project from the monorepo on a Windows Circle CI machine.

Problem

Previously the Verdaccio configuration was set to output the Verdaccio logs to the terminal.
The create-mobify-app-dev.js script read the logs outputted to the terminal, waiting for Verdaccio to be up and running before proceeding with the rest of the promise chain. The script also created a verdaccio.log file with some of the logs printed in the terminal.

The script first publishes the existing versions pwa-kit-create-app and pwa-kit-react-sdk monorepo packages to the local npm repository Verdaccio. Then generates a project from the packages that we just published to the local npm repository using the command npx pwa-kit-create-app and finally runs tests on the generated project.

A more detailed description of the create-mobify-app-dev.js script can be found here.

That approach was not working on a Windows Circle CI machine. The process hung without any output for more than 10 min.
(Optional see failed test on previous commits of this branch).

Potentially the problem was related to the terminal output piping between the different processes run in the promise chain. The Verdaccio logs were overwritten and lost by the following processes output in the promises, ending in a situation where the script didn't output any output and nor the expected non-zero exit by Circle CI.

Solution

The Verdaccio configuration has been updated to output all the Verdaccio's logs to a verdaccio.log file using the native Verdaccio functionality.

The create-mobify-app-dev.js script now monitors the verdaccio.log file content and waits until Verdaccio is ready before continuing with the rest of the promise chain.

The generated project npm install progress is now visible like we did on V2 (BFH branch).

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • Added a new Circle CI job to generate a project from the monorepo on a Windows machine.
  • Refactor a new generate_and_test_project step in the Circle CI config to be used by the test jobs (generatedSFCCProjectTestWindows and generatedSFCCProjectTestWindows) running on Linux and Windows machines.
  • Changed Verdaccio configuration to output the logs to a verdaccio.log file.
  • Updated Verdaccio to the latest v4 version.
  • The create-mobify-app-dev.js script now monitors the verdaccio.log file instead of the terminal.
  • Switched shelljs with Node child_process for some processed that were stuck on Windows.
  • The generated project npm install progress is now visible.

How to Test-Drive This PR

(Optional) Manually run the create-mobify-app-dev.js script.

Mac:

GENERATOR_PRESET=test-project node packages/pwa-kit-create-app/scripts/create-mobify-app-dev.js --outputDir generated-project

Windows:

set GENERATOR_PRESET=test-project
node packages\pwa-kit-create-app\scripts\create-mobify-app-dev.js --outputDir generated-project

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

@adamraya adamraya requested a review from a team as a code owner February 17, 2022 23:03
@adamraya adamraya added the ready for review PR is ready to be reviewed label Feb 25, 2022
GENERATOR_PRESET: 'hello-world-test-project'
command: |
set GENERATOR_PRESET=hello-world-test-project
node packages/pwa-kit-create-app/scripts/create-mobify-app-dev.js --outputDir generated-project-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

@adamraya this looks mostly good to me.

I see that we're generating both test-project and hello-world on windows but we only build and start test-project. Should we be building hello-world after it's generated as well?

Copy link
Collaborator Author

@adamraya adamraya Mar 2, 2022

Choose a reason for hiding this comment

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

@vcua-mobify The test we run here is to verify if we can successfully generate a hello-world project.
i.e. Running the create-mobify-app-dev.js script with the GENERATOR_PRESET=hello-world-test-project environment variable exits with a zero exit.

This is the same test that we currently do on the develop branch:
https://github.com/SalesforceCommerceCloud/pwa-kit/blob/develop/.circleci/config.yml#L283-L288

The change in this PR is that we now run the same test on a Linux machine and on a Windows machine.

The hello-world project is empty, so we don't have anything to test.

If you're saying that we need to test if the hello-world project starts after testing that builds successfully, I think that could be a useful test to have, but that would be out of the scope of this PR, and we'll need to address that in a new ticket.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the answer Adam Yeah, I was wondering if we should be testing the hello-world project starts after it builds but let's save that for another ticket.

@adamraya adamraya merged commit 986e5da into develop Mar 3, 2022
@adamraya adamraya deleted the circle-ci-test-generated-project-windows branch March 3, 2022 00:20
adamraya added a commit that referenced this pull request Mar 22, 2022
* Fix SDK's peer dependencies versioning (#432)

Previously the range was matching all possible versions. Now matching React 16 and 17 only.

* Node warnings with better version formatting (#410)

The node version range is now taken from the engines field of the package.json, which is more readable than seeing semver's representation (>=12.0.0 <13.0.0-0||>=14.0.0 <15.0.0-0).

* [Circle CI] Add test generated project job on Windows (#412)

* Update config.yml

* Add `generatedSFCCProjectTestWindows` test job

* Test not using gtime.js script

* Increase log verbosity

* Run first the CI job generated project on Windows

* Use  outputDir generated-project

* Use execSync

* Update npx pwa-kit-create-app command

* Use process.argv as outputDir

* Test execFileSync

* Add try/catch to execSync

* Show npm i logging

* Show npm install logs as we do in the BFH branch

* Show verdaccio logs

* More logs npm install verbose

* Use Node child_process & Update verdaccio config & bump version

* Use verdaccio npmjs cache true

* Set GENERATOR_PRESET env variable

* Update create-mobify-app.js

* Test using a xlarge Windows machine

* Increase Verdaccio max_fails config

* Remove server keepAliveTimeout

* Dump all Verdaccio logs to a file for debugging

* Fix smoketestscripts on Windows

* Add Circle CI unit to the scripts path

* Really fix smoketestscripts Circle CI job on Windows

* Update config.yml

* More testing smoke test script on Windows

* And more testing smoke test script on Windows

* smoketestscripts job working on Windows

* fix typo

* Clean up

* More clean up

* Update latest v4 verdaccio

* Remove file from bad merge

Co-authored-by: Vincent Marta <vmarta@salesforce.com>
@adamraya adamraya mentioned this pull request Mar 22, 2022
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants