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

Add timeout and test environment app.import call #1619

Merged
merged 5 commits into from
Dec 11, 2024
Merged

Conversation

TylerMatteo
Copy link
Contributor

@TylerMatteo TylerMatteo commented Dec 9, 2024

These changes are intended to resolve two different errors we've been having with running tests for this repo.

  1. The change to testem.js fix an error seen sporadically referring to a timeout being reached. This error would manifest with a reference to Testem not being loaded
  2. The change to ember-cli-build is meant to fix the error seen since Pratishta/subscribe #1563 was merged in. As far as I can tell, this error was due to foundation not being imported when the app was run in test mode. This seems to have fixed that but not 100% sure.

Tests now execute successfully but fail. The error I'm seeing says "Assertion Failed: Attempted to register a view with an id already in use". My working theory is that this is due to use of Foundations modal component introduced in #1559, based on a similar Issue found in Ember's repo.

I'd like to merge this in to fix the existing issues, knowing the tests still fail, then have @dhochbaum-dcp investigate the new error.

@TylerMatteo TylerMatteo requested a review from a team as a code owner December 9, 2024 20:44
Copy link

netlify bot commented Dec 9, 2024

Deploy Preview for labs-zap ready!

Name Link
🔨 Latest commit 96363ea
🔍 Latest deploy log https://app.netlify.com/sites/labs-zap/deploys/6759cd3b4d57c40008da7d2d
😎 Deploy Preview https://deploy-preview-1619--labs-zap.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@TylerMatteo TylerMatteo changed the title WIP Add timeout and test environment app.import call Add timeout and test environment app.import call Dec 10, 2024
@TylerMatteo
Copy link
Contributor Author

@pratishta mind approving if it looks good to you? David and I both have commits so don't want to be approvers.

Copy link
Contributor

@pratishta pratishta left a comment

Choose a reason for hiding this comment

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

lgtm

@dhochbaum-dcp dhochbaum-dcp merged commit 196c1f3 into develop Dec 11, 2024
5 checks passed
@dhochbaum-dcp dhochbaum-dcp deleted the tm-fix-tests branch December 11, 2024 17:42
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.

3 participants