Skip to content

Commit

Permalink
Update CONTRIBUTING.md and support WSL2 (#3378)
Browse files Browse the repository at this point in the history
* Fix WSL2 build

* Refreshed content

* Update

* Add workflows

* Update

* Add lolex

* Update verbage

* Update verbage

* Update verbage

* Update verbage

* Update verbage

* Update content

* Update entry

* Apply suggestions from code review

Co-authored-by: Corina <14900841+corinagum@users.noreply.github.com>

* Apply PR suggestions

* Typo

Co-authored-by: Corina <14900841+corinagum@users.noreply.github.com>
  • Loading branch information
compulim and corinagum authored Aug 10, 2020
1 parent f0d49cc commit 8c422fd
Show file tree
Hide file tree
Showing 7 changed files with 274 additions and 68 deletions.
230 changes: 199 additions & 31 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,64 +8,232 @@ When you submit a pull request, a CLA-bot will automatically determine whether y
a CLA and decorate the PR appropriately (e.g., label, comment). Simply follow the instructions
provided by the bot. You will only need to do this once across all repos using our CLA.

# Building the project
# Thank you

Thanks for your interest in improving Web Chat. We invest heavily in engineering excellence to reduce our workloads and enable us to move faster. To start the development, please familiarize yourself with the development process and these automation tools.

> If you need to build this project for customization purposes, we strongly advise you to refer to our [samples](https://github.com/microsoft/BotFramework-WebChat/tree/master/samples). If you cannot find any samples that fulfill your customization needs and you don't know how to do that, please [send your dream to us](https://github.com/microsoft/BotFramework-WebChat/issues/).
>
> Forking Web Chat to make your own customizations means you will lose access to our latest updates. Maintaining forks also introduces chores that are substantially more complicated than a version bump.
To build Web Chat, you will need to make sure both your Node.js and NPM is latest version (either LTS or current, must be `>= 12`).
# Preparing the environment

There are 2 steps to prepare your environment: [install tools](#installing-tools) and [prepare the repository](#preparing-the-repository).

## Installing tools

Please install the follow in your development environment:

## Preparing the build
- [Node.js and NPM](https://nodejs.org/) of LTS or latest version
- [Docker](https://docs.docker.com/get-docker/)

After you clone the repository, run the following to make sure all dependencies were installed.
These are development environments we tested:

- Windows 10 Professional
- With WSL1 or [WSL2](https://aka.ms/wsl2) installed
- [Ubuntu 18 running under WSL2](https://www.microsoft.com/en-us/p/ubuntu-1804-lts/9n9tngvndl3q)

## Preparing the repository

Web Chat is a monorepo and uses [`lerna`](https://lerna.js.org/). After the repository is [forked and cloned](https://github.com/microsoft/botframework-webchat/fork), run the following to install all dependencies.

```sh
npm ci
npm run bootstrap
```

## Build tasks
# Building the project

You can use either one of the following scripts to build Web Chat:
> Default build flavor is development. Please read [BUILD_SCRIPTS.md](https://github.com/microsoft/BotFramework-WebChat/blob/master/docs/BUILD_SCRIPTS.md) about different build flavors.
- `npm run build` will build once
- `npm start` will build and continuously rebuild if changes are detected
There are two ways to build: one-off and continuous build.

## Trying out the build
- For one-off building, run `npm run build`
- For continuous building, run `npm start`

Web Chat's playground app under `/packages/` is for testing and debugging.
The bundle output will be located at:

```sh
cd packages/playground
npm run start
```
- http://localhost:5000/webchat.js (also `webchat-es5.js` and `webchat-minimal.js`)
- `packages/bundle/dist/webchat*.js`

Then navigate to http://localhost:3000/, and click on one of the connection options on the upper right corner.
# Playing with the build

- Official MockBot: we hosted a live demo bot for testing Web Chat features
- Emulator Core: it will connect to http://localhost:5000/v3/directline for [emulated Direct Line service](https://github.com/microsoft/BotFramework-Emulator/tree/master/packages/emulator/cli/)
There are few ways you can play with the build:

You are also advised to test the CDN bundles by copying the test harness from our samples.
- Playing with playground
- `cd packages/playground`
- `npm start`
- Navigate to http://localhost:3000/, and click on "Official MockBot" on the upper right corner
- Playing with `webchat-loader`
- Navigate to https://compulim.github.io/webchat-loader/
- In the version dropdown, select `http://localhost:5000/webchat-es5.js`
- Write your own web app
- `<script src="http://localhost:5000/webchat.js"></script>`
- Or using tarballs

## Running integration tests
# Readying for publication

```bash
docker-compose up --build --detach
npm test
docker-compose down --rmi all
```
We care about software quality. Quality checking prevents regressions, reduces maintenance costs, minimizes chores, and enables us to move faster.

## Static code analysis
Before submitting the pull request, please check your build by running these checks:

Before committing code, please run the following command:
- [Running automated tests](#running-automated-tests)
- [Performing static code analysis](#static-code-analysis)

```
npm run eslint
```
## Running automated tests

Automated tests in Web Chat are designed to run inside a stable and isolated test environment. To run the test suite, you will need to [start the test environment](#starting-the-test-environment), then [run the test suite](#running-the-test-suite).

### Starting the test environment

eslint and prettier will ensure that your code follows our linting guidelines without too much effort. If you have any questions about this process, please feel free to leave comments in your Pull Request.
For environment stability, Web Chat uses Docker for hosting the test environment.

- Docker with [Windows Subsystem for Linux 2 (a.k.a. WSL2)](https://aka.ms/wsl2)
- To start, run `npm run start:docker`
- To stop/cleanup, run `npm run stop:docker`
- Other Docker setup
- To start, run `docker-compose up --build`
- To stop/cleanup, run `docker-compose down`

> On Windows, set environment variable `COMPOSE_CONVERT_WINDOWS_PATHS=1`.
### Running the test suite

> Testing pages under `__tests__/html/` are served under http://localhost:5001/ (locally) and http://localhost:5081/ (from Docker). All Jest tests will run against Docker version of the page. To update the page, restart Docker.
There are 2 ways to run the test suite, either one-off or continuously:

- For one-off testing, `node_modules/.bin/jest`
- For continuous testing, `npm test`
- To speed up continuous testing, you can install [watchman](https://facebook.github.io/watchman/)

If your development box has less than 4 cores, you will need to reduce the number of simultaneous test agents:

- For one-off testing: run `node_modules/.bin/jest --maxWorkers 1`
- For continuous testing: run `npm test -- --maxWorkers 1`

Our CI pipeline run tests with 4 agents simultaneously. If new tests are added, please make sure they can run simultaneously.

> To run Web Driver test, locally, download [ChromeDriver](https://sites.google.com/a/chromium.org/chromedriver/downloads) and extract to project root. Then set environment variable `WEBCHAT_TEST_ENV=chrome-local` before running Jest.
### Troubleshooting the test suite

We run test suite on every commit and requires 100% test pass. If the test suite did not complete successfully, they are likely:

- Intermittent services issue
- Test reliability issue, please see [#2938](https://github.com/microsoft/BotFramework-WebChat/issues/2938)
- Polluted development environment, for example:
- Outdated `node_modules` content
- Outdated Node.js or NPM

When the test suite failed:

1. Re-run the test suite
1. If it succeeded but a specific test fails intermittently, please comment to [#2938](https://github.com/microsoft/BotFramework-WebChat/issues/2938)
1. If it continues to fail, please fresh clone the repository and run the test suite without your changes
1. If you suspect your environment is polluted, please use [Windows Sandbox](https://docs.microsoft.com/en-us/windows/security/threat-protection/windows-sandbox/windows-sandbox-overview) and/or a new Linux distro on WSL to verify
1. If it still fails repeatedly, please [file an issue](https://github.com/microsoft/BotFramework-WebChat/issues/new/choose) to us.

## Static code analysis

## Adding languages
Run `npm run eslint` for static code analysis.

If you need to skip any ESLint errors, we recommend `eslint-disable-next-line`. Comments are required on why the rule is disabled.

# Final checks

There are checks that automation will not be able to capture. For example:

- Hygiene

- Add a new log to `CHANGELOG.md`
- Fill out the pull request form for traceability
- Make sure imports, members, variables, etc, are sorted alphabetically
- Avoid one-off variables, prefer JavaScript shorthands, shorter and faster code
- No global pollution:
- no specific tab order,
- minimize `z-index` usage
- if z-index is ultimately required, use a [new stacking context](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context),
- all polyfills must comply with ECMAScript/W3C standards

- Tests
- Tests are important to reduces our maintenence burden. Pull requests without needed tests will not be approved or merged into the project.
- When fixing a bug, the original bug repro must be included as a new test
- For feature work, please add as many tests as needed to future-proof the feature, including both happy and unhappy paths
- We prefer integration tests over unit tests
- Visual regression tests are preferred and we use [`pixelmatch`](https://npmjs.com/package/pixelmatch) via [`jest-image-snapshot`](https://npmjs.com/package/jest-image-snapshot)
- Avoid using timeout in tests. Instead, use [fake timer](https://www.npmjs.com/package/lolex) instead
- [Secure by default](https://en.wikipedia.org/wiki/Secure_by_default)
- Benchmark
- Performance should not drop significantly
- Bundle size should not increase significantly
- Code coverage should not decrease significantly
- Backward compatibility should be maintained for 2 years
- Cross browser compatibility
- Windows 10
- Chrome
- Edge Chromium
- Edge UWP
- Firefox
- IE11 (except speech features)
- Safari on macOS
- Safari on iOS or iPadOS
- Chrome on Android
- For accessibility, please refer to [`docs/ACCESSIBILITY.md`](https://github.com/microsoft/BotFramework-WebChat/blob/master/docs/ACCESSIBILITY.md)
- Tab order, content readability, assistive technology-only text, color contrast, etc. must be maintained.
- Assistive technology and browser compatibility
- NVDA/JAWS: Chrome and Firefox
- Narrator: Edge family and IE11
- VoiceOver: Safari
- TalkBack: Chrome on Android
- For internationalization, please refer to [`docs/LOCALIZATION.md`](https://github.com/microsoft/BotFramework-WebChat/tree/master/docs/LOCALIZATION.md)
- Feature documentation, samples, live demo, operation of demo bots
- All samples must also come with a hosted live demo
- Please discuss with us if a specific bot is needed for the live demo

# Workflow

Here list how we generally work when [fixing a bug](#fixing-bug) or [implementing a new feature](#implement-new-feature).

## Fixing bug

Write the bug repro as a test, before fixing the bug.

1. Clone/prepare the repository or nuke an existing repository
- To nuke, run `npm run tableflip`
1. Run continuous build, run `npm start`
1. Convert bug into a test case, under `__tests__/html/this-is-the-bug.html`
1. Run the test, `npm run start:docker`, then `npm test -- --testPathPattern html/this-is-the-bug.html`
- Make sure the test fail as described in the repro
1. Fix the bug
1. Re-run `npm run start:docker` to refresh the code inside Docker
1. Re-run the test, it should succeed
- Verify screenshots if any
1. Add a log entry to `CHANGELOG.md`
1. File a pull request

## Implementing new features

Write the user story while implementing the feature.

1. Clone/prepare the repository or nuke an existing repository
- To nuke, run `npm run tableflip`
1. Run continuous build, run `npm start`
1. Clone `__tests__/html/simple.html` to `my-feature.html` and use it as a playground
1. Navigate to the new playground at http://localhost:5001/my-feature.html
1. Implement the feature while modifying the playground
1. After the feature is completed, write more tests to include cases that need more attention, and unhappy paths
1. Run the test, `npm run start:docker`, then `npm test -- --testPathPattern html/my-feature*.html`
- Verify screenshots
1. Write a new sample with `README.md`
- This is the user story and proof-of-record on how the feature will work
- Update [`samples/README.md`](https://github.com/microsoft/BotFramework-WebChat/blob/master/samples/README.md)
1. Add more design docs to [`/docs`](https://github.com/microsoft/BotFramework-WebChat/tree/master/docs) as needed
1. Add a few log entries to `CHANGELOG.md`
1. File a pull request

## Additional context

### Adding languages

To add a new language to our localization list, please refer to [`docs/LOCALIZATION.md`](https://github.com/microsoft/BotFramework-WebChat/tree/master/docs/LOCALIZATION.md).
36 changes: 29 additions & 7 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,40 @@
Fixes #<!-- If this addresses a specific issue, please provide the issue number here -->
<!-- Please provide the issue number here if any -->

> Fixes #
## Changelog Entry

<!-- Please paste your new entry from CHANGELOG.MD here -->
<!-- Please paste your new entry from CHANGELOG.MD here. Entry is not required for work only related to development purposes. -->

## Description

<!-- Please discuss the changes you have worked on. What do the changes do; why is this PR needed? -->
<!-- Please discuss the changes you have worked on. What do the changes do; why is this PR needed? -->

## Design

<!-- If this feature is complicated in nature, please provide additional clarifications. -->

## Specific Changes

<!-- Please list the changes in a concise manner. -->
<!-- Please list the changes in a concise manner. -->

-
-
-

<!-- For bugs, add the bug repro as a test. Otherwise, add tests to futureproof your work. -->
- [ ] I have added tests and executed them locally
- [ ] I have updated `CHANGELOG.md`
- [ ] I have updated documentation

## Review Checklist

---
> This section is for contributors to review your work.
- [ ] Testing Added
<!-- If you are adding a new feature to a library, you must include tests for your new code. -->
- [ ] Accessibility reviewed (tab order, content readability, alt text, color contrast)
- [ ] Browser and platform compatibilities reviewed
- [ ] CSS styles reviewed (minimal rules, no `z-index`)
- [ ] Documents reviewed (docs, samples, live demo)
- [ ] Internationalization reviewed (strings, unit formatting)
- [ ] `package.json` and `package-lock.json` reviewed
- [ ] Tests reviewed (coverage, legitimacy)
29 changes: 1 addition & 28 deletions __tests__/README.md
Original file line number Diff line number Diff line change
@@ -1,28 +1 @@
## How to run tests

Automated testing in Web Chat is using multiple open-source technologies.

- [Azure DevOps](https://azure.microsoft.com/en-us/services/devops/) for automatic testing
- Test against [MockBot](https://github.com/compulim/BotFramework-MockBot)
- Try it out with this [live demo](https://microsoft.github.io/BotFramework-WebChat/01.getting-started/a.full-bundle)
- Visual regression tests (a.k.a. compare screenshots)
- Generated on [Chrome on Docker](https://github.com/SeleniumHQ/docker-selenium)
- Compared using [`pixelmatch`](https://npmjs.com/package/pixelmatch) via [`jest-image-snapshot`](https://npmjs.com/package/jest-image-snapshot)
- Run under [`Jest`](https://jestjs.io/)
- [`Istanbul`](https://npmjs.com/package/istanbul) for code coverage
- [`Coveralls`](https://coveralls.io/) for test statistics

### Running tests under Docker

- Install Docker
- On Windows, set environment variable `COMPOSE_CONVERT_WINDOWS_PATHS=1`
- `docker-compose up --build`
- In a separate terminal, run:
- `npm test`

### Running tests under local box

- Install latest Chrome
- Download [ChromeDriver](https://sites.google.com/a/chromium.org/chromedriver/downloads) and extract to project root
- Set environment variable `WEBCHAT_TEST_ENV=chrome-local`
- `npm test`
Please refer to [CONTRIBUTING.md](https://github.com/microsoft/BotFramework-WebChat/tree/master/.github/CONTRIBUTING.md#running-automated-tests) on running automated tests.
39 changes: 39 additions & 0 deletions docker-compose-wsl2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
version: '3'

services:
# On Windows, run with COMPOSE_CONVERT_WINDOWS_PATHS=1

chrome:
build:
context: ./
dockerfile: Dockerfile-selenium
depends_on:
- selenium-hub
- webchat
- webchat2
environment:
- HUB_HOST=selenium-hub
- HUB_PORT=4444

selenium-hub:
image: selenium/hub:3.141.59-zirconium
container_name: selenium-hub
environment:
- GRID_BROWSER_TIMEOUT=60
- GRID_TIMEOUT=60
ports:
- "4444:4444"

webchat:
build:
context: ./
dockerfile: Dockerfile-testharness
ports:
- "5080:80"

webchat2:
build:
context: ./
dockerfile: Dockerfile-testharness2
ports:
- "5081:80"
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
"posteslint": "npm run prettier-readmes",
"prettier-readmes": "prettier --write **/**/*.md --tab-width 3 --single-quote true",
"start": "concurrently --kill-others --raw \"serve\" \"serve -p 5001 -c serve-test.json\" \"lerna run --ignore playground --parallel --stream start\"",
"start:docker": "npm run start:docker:build && npm run start:docker:up",
"start:docker:build": "docker-compose -f docker-compose-wsl2.yml build --parallel",
"start:docker:up": "docker-compose -f docker-compose-wsl2.yml up --scale chrome=4",
"stop:docker": "docker-compose -f docker-compose-wsl2.yml stop",
"tableflip": "npx lerna clean --yes --concurrency 8 && npx rimraf node_modules && npm ci && npm run bootstrap -- --concurrency 8",
"test": "jest --watch",
"test:ci": "npm run test -- --ci --coverage true --no-watch",
Expand Down
Loading

0 comments on commit 8c422fd

Please sign in to comment.