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

chore: improve steps for running 'npm test' #82

Merged
merged 1 commit into from Jul 18, 2019

Conversation

@emonddr
Copy link
Contributor

commented Jul 18, 2019

Description

When running 'npm test' locally it is important to set some environment variables.

The value of DASHDB_SCHEMA must be set to a specific pattern; otherwise certain tests fail because some cleanup doesn't occur beforehand.

Or other variables

PACKAGE_NAME 
BUILD_NUMBER
nodeVersion

can be set to compute the schema name ( following the proper pattern) for the user.

This is how the Jenkins job sets up the environment variables before running npm test, and the
user should do it to see the same results.

Big thanks to @b-admike for helping me debug this.

It solves the failure

1) Discover and build models
       should discover and build models:

      Uncaught AssertionError [ERR_ASSERTION]: error should not be reported
      + expected - actual

      -false
      +true

I experienced when running npm test locally. The value I specified for DASHDB_SCHEMA didn't follow the pattern and was causing the failure.

With the env variables properly set, all tests are passing.

Related issues

connect to #76

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

@emonddr emonddr requested review from nabdelgadir and removed request for virkt25 Jul 18, 2019

@emonddr emonddr force-pushed the dremond_improve_readme branch from cce8f23 to 8f3f5b8 Jul 18, 2019

@emonddr

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

This is where the schema name is computed following the pattern:

https://github.com/strongloop/loopback-connector-dashdb/blob/master/test/init.js#L20

This is the clean up code, which uses same schema name pattern:

https://github.com/strongloop/loopback-connector-dashdb/blob/master/pretest.js#L23

@b-admike
Copy link
Member

left a comment

Nice 👍

@emonddr emonddr merged commit 569f7f1 into master Jul 18, 2019

8 checks passed

Commit Linter commits are all properly formatted
Details
PR Linter PR is up to date
Details
[cis-jenkins] PR Builder Build finished.
Details
[cis-jenkins] x64 && linux && nvm,10 Success! (8f3f5b8)
Details
[cis-jenkins] x64 && linux && nvm,6 Success! (8f3f5b8)
Details
[cis-jenkins] x64 && linux && nvm,8 Success! (8f3f5b8)
Details
clahub All contributors have signed the Contributor License Agreement.
Details
security/snyk - package.json (StrongLoop) No manifest changes detected

@delete-merged-branch delete-merged-branch bot deleted the dremond_improve_readme branch Jul 18, 2019


This will create a schema with the name: **SCHEMA1_DASHDB_DARWIN_10**

This pattern of the schema name is important for database **cleanup** and **seeding** purposes.

This comment has been minimized.

Copy link
@bajtos

bajtos Jul 18, 2019

Member

+1 to update the README to describe all necessary steps.

I am confused though - why is it necessary to provide all of these properties manually? Why cannot our test suite infer them automatically?

  • Read PACKAGE_NAME from package.json's property name
  • Set BUILD_NUMBER to arbitrary value (e.g. 1`)
  • Set nodeVersion to the value returned by process.versions.node.split('.')[0] (JavaScript code to be executed in Node.js land)

This comment has been minimized.

Copy link
@b-admike

b-admike Jul 18, 2019

Member

I think this is great for future enhancement. IIRC the pretest was originally created to set these values up using env variables set by Jenkins. @bajtos @emonddr do we want to create a follow up issue for it?

This comment has been minimized.

Copy link
@emonddr

emonddr Jul 18, 2019

Author Contributor

@bajtos @b-admike @dhmlau , I opened an issue for this future enhancement : #83

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.