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.

Sign up for GitHub

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

Jump to bottom

Modal mfs e2e tests #460

Merged
merged 4 commits into from
Apr 9, 2019
Merged

Modal mfs e2e tests #460

merged 4 commits into from
Apr 9, 2019

Conversation

parostatkiem-zz
Copy link
Contributor

@parostatkiem-zz parostatkiem-zz commented Apr 3, 2019 •
edited
Loading

Description

Changes proposed in this pull request:

  • add e2e tests
  • add data-e2e selector modal

Related issue(s)

@parostatkiem-zz parostatkiem-zz added enhancement New feature or request area/luigi labels Apr 3, 2019
@y-kkamil y-kkamil self-assigned this Apr 4, 2019
y-kkamil
y-kkamil suggested changes Apr 4, 2019
View reviewed changes
Copy link

@y-kkamil y-kkamil left a comment

Choose a reason for hiding this comment

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

looks like something is wrong, fails with
CypressError: Timed out retrying: Expected to find element: '[data-e2e=modal-mf]', but never found it.
for three tests:

  • can be closed by Close Button
  • sets proper URL inside iframe
  • has the size set directly

@parostatkiem-zz
Copy link
Contributor Author

parostatkiem-zz commented Apr 5, 2019 •
edited
Loading

@y-kkamil are you sure your Core is bundled and connected to AngularExample?
For me, it looks like it is not. Have a look at the code of this PR.
I've got them all green and no uncommitted changes 🥗
image

y-kkamil
y-kkamil approved these changes Apr 5, 2019
View reviewed changes
Copy link

@y-kkamil y-kkamil left a comment

Choose a reason for hiding this comment

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

one small nitpick - i would change the filename to sth like modal-microfrontends.spec.js as we don't use uppercase characters for resources anywhere

@JohannesDoberer JohannesDoberer self-assigned this Apr 9, 2019
@parostatkiem-zz parostatkiem-zz merged commit 619549e into SAP:master Apr 9, 2019
@parostatkiem-zz parostatkiem-zz deleted the modal-MFS-e2e-tests branch April 9, 2019 11:14
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
@parostatkiem
Modal mfs e2e tests (SAP#460)
791e907 
* Add data-e2e attr to modal

* Add modalMF test file

* Rename the test file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@y-kkamil y-kkamil y-kkamil approved these changes

@JohannesDoberer JohannesDoberer JohannesDoberer approved these changes

@antiheld antiheld Awaiting requested review from antiheld

@dariadomagala-sap dariadomagala-sap Awaiting requested review from dariadomagala-sap

@hardl hardl Awaiting requested review from hardl hardl is a code owner

@jesusreal jesusreal Awaiting requested review from jesusreal

@kwiatekus kwiatekus Awaiting requested review from kwiatekus

@marynaKhromova marynaKhromova Awaiting requested review from marynaKhromova

@maxmarkus maxmarkus Awaiting requested review from maxmarkus

@pekura pekura Awaiting requested review from pekura

Assignees

@y-kkamil y-kkamil

@JohannesDoberer JohannesDoberer

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@parostatkiem-zz @y-kkamil @JohannesDoberer