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

feat(cypress): add cypress_web_test rule and @bazel/cypress package #1930

Merged
merged 1 commit into from
Jul 6, 2020

Conversation

mrmeku
Copy link
Collaborator

@mrmeku mrmeku commented May 31, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Add @bazel/cypress for integrating cypress testing framework with bazel

Left todo:
Show how to launch a nodejs_binary server from cypress plugin

@mrmeku mrmeku force-pushed the cypress branch 9 times, most recently from c78ee84 to a8784b2 Compare June 6, 2020 03:54
@mrmeku mrmeku marked this pull request as draft June 12, 2020 19:20
@mrmeku mrmeku force-pushed the cypress branch 16 times, most recently from 0d05d8c to eed4a00 Compare June 14, 2020 21:14
@mrmeku mrmeku force-pushed the cypress branch 3 times, most recently from 28fa367 to c2287c7 Compare June 16, 2020 04:21
@mrmeku mrmeku changed the title WIP: feat(cypress): add cypress_web_test rule and @bazel/cypress package feat(cypress): add cypress_web_test rule and @bazel/cypress package Jun 16, 2020
@mrmeku mrmeku marked this pull request as ready for review June 16, 2020 19:48
@mrmeku mrmeku requested a review from mattem as a code owner June 16, 2020 19:48
@mrmeku
Copy link
Collaborator Author

mrmeku commented Jun 23, 2020

@alexeagle @gregmagolan time for a review! :D

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

should have a codeowners change too, so you can review changes here

.bazelrc Show resolved Hide resolved
.bazelci/presubmit.yml Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
WORKSPACE Outdated Show resolved Hide resolved
examples/cypress/.bazelrc Show resolved Hide resolved
],
)

ts_library(
Copy link
Collaborator

Choose a reason for hiding this comment

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

any particular reason you chose ts_library rather than ts_project? I expect most users will prefer the latter since it's compatible with their existing code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason I opted for ts_library was to test that I consumed the ts_library providers correctly. Maybe I should use both a ts_library and ts_project? What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

we shouldn't have a difference between providers, such that you can tell whether ts_project or ts_library was used to compile your dependency. Could you file an issue with the details about what's different? I hope you don't still need the "typescript" provider

@@ -0,0 +1,6 @@
describe('world', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is there a .ts and .js file both checked in? I suspect this one doesn't belong

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did so to show how to load a .js file as a test and as a .ts file as a test respectively

package.json Outdated Show resolved Hide resolved
packages/cypress/BUILD.bazel Outdated Show resolved Hide resolved
packages/cypress/internal/cypress-install.js Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@mrmeku mrmeku force-pushed the cypress branch 6 times, most recently from 308f9de to c5f806b Compare June 24, 2020 22:35
@mrmeku mrmeku requested a review from alexeagle June 24, 2020 22:37
@mrmeku mrmeku force-pushed the cypress branch 7 times, most recently from 7381a43 to 4872e4d Compare June 25, 2020 19:05
Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

Yay I think this looks great. I don't totally follow all the details how how cypress installs as a workspace but I trust that you'll be around to help if we need it :)

],
)

ts_library(
Copy link
Collaborator

Choose a reason for hiding this comment

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

we shouldn't have a difference between providers, such that you can tell whether ts_project or ts_library was used to compile your dependency. Could you file an issue with the details about what's different? I hope you don't still need the "typescript" provider

@alexeagle alexeagle merged commit 3bac870 into bazel-contrib:master Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants