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

Feature/439767 - Configure e2e tests for next.js. #523

Merged
merged 6 commits into from Dec 30, 2020

Conversation

sc-olexandrtroyanovskyy
Copy link
Collaborator

Description

Motivation

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update (non-breaking change; modified files are limited to the /docs directory)

Checklist:

  • I have read the Contributing guide.
  • My code follows the code style of this project.
  • My code/comments/docs fully adhere to the Code of Conduct.
  • My change is a code change and it requires an update to the documentation.
  • My change is a documentation change and it requires an update to the navigation.

e2e/cypress/package.json Outdated Show resolved Hide resolved
@@ -19,6 +19,13 @@ if (!verb || (verb !== 'connected' && verb !== 'disconnected')) {
}

const config = [
// Due to current inablitiy to set\change a port for the next.js app
Copy link
Contributor

Choose a reason for hiding this comment

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

The Next.js CLI supports a port parameter for the dev and start commands, which should be able to read an env variable. What if you change the Next.js sample app's "next:dev" to:

"next:dev": "cross-env NODE_OPTIONS='--inspect' next dev --port $PORT",

It seems to use 3000 by default if PORT isn't defined (which is what we'd want), but should pick up our e2e setting I think?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ambrauer If you use cross-env it will not pick up PORT env variable, I tried to do it (if you use appmodes.js script).
In case If I use cross-env-shell NODE_OPTIONS='--inspect' next dev --port $PORT it will work only if $PORT is defined, if this variable is not provided, it will throw error: UnhandledPromiseRejectionWarning: Error: Option requires argument: --port

Copy link
Contributor

Choose a reason for hiding this comment

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

@sc-illiakovalenko That's a bummer. I attempted to use a fork of cross-env which adds default value functionality. That solved the "option requires argument" error, however didn't pick up the port supplied by appmodes.js.

I suppose the other option would be to rewrite appmodes.js to fire up and test each app sequentially - avoiding any port conflicts - but that's beyond the scope of this task.

@sc-illiakovalenko sc-illiakovalenko merged commit aecb9ac into dev Dec 30, 2020
@sc-illiakovalenko sc-illiakovalenko deleted the feature/439767 branch December 30, 2020 15:14
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

3 participants