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

fix(cli): correct root directory in test #4652

Merged
merged 1 commit into from Feb 14, 2020

Conversation

@dougal83
Copy link
Contributor

dougal83 commented Feb 14, 2020

Fixes error on windows tests. Removes instances of sandbox folder
erroniously placed in packages folder.

See #4425

Signed-off-by: Douglas McConnachie dougal83+git@gmail.com

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
Fixes error on windows tests.  Removes instances of sandbox folder
erroniously placed in packages folder.

See #4425

Signed-off-by: Douglas McConnachie <dougal83+git@gmail.com>
@dougal83 dougal83 requested a review from bajtos Feb 14, 2020
@dougal83 dougal83 added the os:Windows label Feb 14, 2020
@dougal83 dougal83 requested a review from agnes512 Feb 14, 2020
@bajtos

This comment has been minimized.

Copy link
Member

bajtos commented Feb 14, 2020

Thank you for the pull request. I think it may be an intentional decision to place the sandbox directory into packages, because that way lerna bootstrap will install dependencies using monorepo-local links to resolve LB4 packages. I don't know if this is applicable to the particular tests you are modifying.

AFAICT, one of the problematic tests was introduced by 6302101 by @agnes512.

Agnes, do you happen to remember what was your intention? What is the correct rootDir value to use?

@agnes512

This comment has been minimized.

Copy link
Contributor

agnes512 commented Feb 14, 2020

I think @dougal83 is right. The test was supposed to be tested in sandbox. I made a mistake, my bad D:

@raymondfeng raymondfeng merged commit f951052 into strongloop:master Feb 14, 2020
4 checks passed
4 checks passed
Travis CI - Pull Request Build Passed
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage remained the same at 92.503%
Details
@dougal83 dougal83 deleted the dougal83:fix-cli-test-root-dir branch Feb 15, 2020
dougal83 added a commit to dougal83/loopback-next that referenced this pull request Feb 15, 2020
ensure test app is remove afterward

follows: strongloop#4652
Signed-off-by: Douglas McConnachie <dougal83+git@gmail.com>
@dougal83 dougal83 mentioned this pull request Feb 15, 2020
3 of 3 tasks complete
dougal83 added a commit to dougal83/loopback-next that referenced this pull request Feb 15, 2020
ensure test app is remove afterward

follows: strongloop#4652
Signed-off-by: Douglas McConnachie <dougal83+git@gmail.com>
dougal83 added a commit to dougal83/loopback-next that referenced this pull request Feb 17, 2020
ensure test app is remove afterward

follows: strongloop#4652
Signed-off-by: Douglas McConnachie <dougal83+git@gmail.com>
raymondfeng added a commit that referenced this pull request Feb 17, 2020
ensure test app is remove afterward

follows: #4652
Signed-off-by: Douglas McConnachie <dougal83+git@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.