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

Fixing some tests that fail because of environment setups. #2590

Closed
wants to merge 4 commits into from
Closed

Conversation

sainthkh
Copy link
Contributor

↪️ Pull Request

Sometimes, we cannot pass some tests because:

  • Java isn't installed or configured. Then, Kotlin test fails. (It's really rare for developers' computer to not have Java. Unfortunately, mine is one of them...)
  • symlinkSync throws EPERM error. On Windows 10, creating symlinks is disabled for security.

So, I changed these:

  • Ignore Kotlin test when there is no Java. I borrowed structure and message from Rust.
  • Show warning message and instruction link to the cmd Admin article when symlink fails.

💻 Examples

None

🚨 Test instructions

  1. git clone https://github.com/parcel-bundler/parcel.git on Windows 10.
  2. yarn test
  3. Test fails.

✔️ PR Todo

  • [V] Added/updated unit tests for this change
  • [V] Filled out test instructions (In case there aren't any unit tests)
  • [] Included links to related issues/PRs

DeMoorJasper
DeMoorJasper previously approved these changes Jan 29, 2019
@@ -27,5 +27,14 @@ function jsonToError(json) {
}
}

function symlinkPrivilegeWarning() {
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this should be part of utils.
We should probably make a utils package for testing Parcel and Parcel plugins somewhere in the future to prevent installing test specific utils to every users download of Parcel.

For now this is good enough though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you know, parcel-bundler/test/utils.js and integration-tests/test/utils.js are exactly same. Because of that, I wondered what to do.

Now I found the test-util package. How about moving this to that package?

Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea, didn't know there was a test-utils package already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a question:

How about merging parcel-bundler/test/utils.js and integration-tests/test/utils.js to test-utils/utils.js this time?

@sainthkh sainthkh closed this Jan 30, 2019
@sainthkh sainthkh reopened this Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants