Skip to content
This repository has been archived by the owner on Jan 5, 2022. It is now read-only.

Add example tests when init generates a new express project #5

Merged
merged 51 commits into from
Oct 22, 2019

Conversation

FrankEssenberger
Copy link
Contributor

@FrankEssenberger FrankEssenberger commented Oct 16, 2019

Proposed Changes

If a fresh express project is created also two tests are added:

  • sample integration test: Starting the express app locally and doing a ping
  • sample unit test: Just a simple assertion

Also two jest configs are included for the unit and integration tests. The configs mainly set the reporting diretory to the S4hana_pipeline so that everything is in line with the pipeline.

Type of Changes

  • Bug Fix
  • New Feature
  • Documentation Update

Checklist

  • I have added tests that prove my fix is effective or that my feature works
    - [ ] I have added necessary documentation

# Conflicts:
#	.github/workflows/nodejs.yml
#	.gitignore
#	.prettierrc
#	LICENSE
#	NOTICE
#	README.md
#	bin/run.cmd
#	jest.config.js
#	package-lock.json
#	package.json
#	src/commands/add-approuter.ts
#	src/commands/add-cx-server.ts
#	src/commands/help-page.ts
#	src/commands/init.ts
#	src/commands/package.ts
#	src/index.ts
#	src/templates/add-approuter/approuter/.npmrc
#	src/templates/add-approuter/approuter/manifest.yml.mu
#	src/templates/add-approuter/approuter/package.json
#	src/templates/add-approuter/approuter/xs-app.json.mu
#	src/templates/add-approuter/approuter/xs-security.json.mu
#	src/templates/add-cx-server/cx-server/cx-server
#	src/templates/add-cx-server/cx-server/cx-server.bat
#	src/templates/add-cx-server/cx-server/server.cfg
#	src/templates/init/.npmrc
#	src/templates/init/Jenkinsfile
#	src/templates/init/credentials.json
#	src/templates/init/deployment/.gitkeep
#	src/templates/init/manifest.yml.mu
#	src/templates/init/pipeline_config.yml.mu
#	src/templates/init/s4hana_pipeline/reports/.gitkeep
#	src/templates/init/s4hana_pipeline/reports/coverage-reports/.gitkeep
#	src/templates/init/systems.json
#	src/utils/copy-list.ts
#	src/utils/templates.ts
#	test/add-approuter.spec.ts
#	test/add-cx-server.spec.ts
#	test/e2e.spec.ts
#	test/express/app.js
#	test/express/package.json
#	test/express/public/stylesheets/style.css
#	test/express/routes/index.js
#	test/express/routes/users.js
#	test/express/views/error.jade
#	test/express/views/index.jade
#	test/express/views/layout.jade
#	test/help-page.spec.ts
#	test/init.spec.ts
#	test/nest/.gitignore
#	test/nest/.prettierrc
#	test/nest/README.md
#	test/nest/nest-cli.json
#	test/nest/package-lock.json
#	test/nest/package.json
#	test/nest/src/app.controller.spec.ts
#	test/nest/src/app.controller.ts
#	test/nest/src/app.module.ts
#	test/nest/src/app.service.ts
#	test/nest/src/main.ts
#	test/nest/test/app.e2e-spec.ts
#	test/nest/test/jest-e2e.json
#	test/nest/tsconfig.build.json
#	test/nest/tsconfig.json
#	test/nest/tslint.json
#	test/package.spec.ts
#	test/tsconfig.json
#	test/tslint.json
#	tsconfig.json
#	tslint.json
@CLAassistant
Copy link

CLAassistant commented Oct 16, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@florian-richter florian-richter left a comment

Choose a reason for hiding this comment

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

Please consider my comments. Especially the TS vs JS for the express tests.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/utils/initialization-helper.ts Outdated Show resolved Hide resolved
src/utils/templates.ts Outdated Show resolved Hide resolved
test/init.spec.ts Outdated Show resolved Hide resolved
test/init.spec.ts Outdated Show resolved Hide resolved
test/init.spec.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@florian-richter florian-richter left a comment

Choose a reason for hiding this comment

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

A minor point concerning the folder structure in the express project: I would prefer if all tests were in a test folder and had subfolders for integration and unit.

src/templates/init/jest.config.js Outdated Show resolved Hide resolved
src/templates/init/test/sample-unit-test.spec.js Outdated Show resolved Hide resolved
src/templates/init/test/test-file-info.ts Outdated Show resolved Hide resolved
src/commands/init.ts Outdated Show resolved Hide resolved
FrankEssenberger and others added 5 commits October 21, 2019 16:24
…stsFreshExpress

# Conflicts:
#	jest.config.js
#	src/commands/init.ts
#	src/templates/init/jest.config.js
#	src/templates/init/jest.integration-test.config.js
#	src/templates/init/jest.unit-test.config.js
#	src/templates/init/test/test-file-info.ts
Comment on lines 68 to 72
const excludes = initializationType === InitializationType.existingProject ? ['test'] : [];
const files = readTemplates({
from: [path.resolve(__dirname, '..', 'templates', 'init')],
to: flags.projectDir,
exclude: excludes
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor thing but I would shorten initializationType to initType (same for the uppercase one) and move the initType === InitType.existingProject ? ['test'] : []; to line 72 so

exclude: initType === InitType.existingProject ? ['test'] : []

@@ -103,31 +95,43 @@ export default class Init extends Command {
}
}

private async initExpressProject() {
const { flags } = this.parse(Init);
private async determineInitializationType(flags: OutputFlags<typeof Init.flags>): Promise<InitializationType> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume there are testing reasons why you pass flags as param instead of putting const { flags } = this.parse(Init); into the function?

I like passing the flags generally, but the type def does look nice. If we keep it as the parameters we should be consistent between all the methods and also put something like type Flags = OutputFlags<typeof Init.flags> somewhere to shorten the param def.

Copy link
Contributor

@florian-richter florian-richter left a comment

Choose a reason for hiding this comment

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

Very nice 🎉

@florian-richter florian-richter merged commit 61a895f into master Oct 22, 2019
@florian-richter florian-richter deleted the addTestsFreshExpress branch October 22, 2019 12:33
@florian-richter florian-richter changed the title Add tests fresh express Add example tests when init generates a new express project Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants