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 e2e tests #4229

Closed
wants to merge 6 commits into from
Closed

fix e2e tests #4229

wants to merge 6 commits into from

Conversation

scsaba
Copy link
Contributor

@scsaba scsaba commented May 9, 2018

No description provided.

@scsaba
Copy link
Contributor Author

scsaba commented May 9, 2018

Hello @danfinlay @kumavis

I noticed that your e2e and screen tests were failing, so I spent a few days to figure out a workaround to bring them to life.

The solution is not the nicest, but maybe this is the least error prone way to get the development time extension id - without changing the manifest.json. This change has much lower risk as it cannot ruin the release package.

@danfinlay
Copy link
Contributor

Oh very nice, this may fix the test failure in #4199. We'll review this today.

@scsaba
Copy link
Contributor Author

scsaba commented May 9, 2018

Meanwhile I could come up with a simpler solution. I will update the pull request in a few hours.

@scsaba
Copy link
Contributor Author

scsaba commented May 10, 2018

@danfinlay updated with the less complicated solution. It has a little risk, because /deep/ css combinator is deprecated. I can see no warning though. If it stops working later then it can be replaced with the other solution.

@danfinlay
Copy link
Contributor

Thanks so much for this contribution.

It turns out some other team members were working on the same issue, and have fixed it similarly in PR #4226, so I'm closing this for now, since it appears to be redundant.

Thank you so much for jumping in and doing this, though!

@danfinlay danfinlay closed this May 10, 2018
@kumavis kumavis mentioned this pull request May 10, 2018
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.

2 participants