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

Tracking Issue [Test Failures] #833

Closed
stefanpenner opened this issue Jun 2, 2021 · 16 comments · Fixed by #839
Closed

Tracking Issue [Test Failures] #833

stefanpenner opened this issue Jun 2, 2021 · 16 comments · Fixed by #839

Comments

@stefanpenner
Copy link
Collaborator

stefanpenner commented Jun 2, 2021

This is an issue to help keep track of/coordinate/ultimately resolve test failures seen. @rwjblue and myself are pairing on this, and we using this issue to keep track of what is up.

Reproduction:

cd embroider/test-packages/macro-sample-addon
yarn test

Results in:

scenario entry status TL;DR current status
ember-lts-2.16
ember-lts-2.20 TypeError: block is not iterable proposed fix #839
ember-lts-2.24 TypeError: block is not iterable proposed fix #839
ember-release Uncaught ReferenceError: Ember is not defined ✅fix landed
ember-beta Uncaught ReferenceError: Ember is not defined ✅fix landed
ember-canary Uncaught ReferenceError: Ember is not defined ✅fix landed

Issues

At first glance, it appears we have two different issues:

1) block is not iterable

Maybe a template compiler iter-opt issue which appears to be at-play for LTS 2.20 -> LTS 2.24, we haven't ruled out it isn't effecting ember-release/ember-beta/ember-canary yet, but as those are failing for other reasons we do not yet know for sure they are not effected.

2) Ember is not defined

Maybe ember's boot-straping of window.Ember isn't running, this appears to effect ember-* scenarios.

Potentially related to: emberjs/ember.js@de777ec / emberjs/ember.js#19557

@stefanpenner
Copy link
Collaborator Author

re 2) Ember is not defined I believe requires a fix in ember, @rwjblue and myself are pairing on that now (more details)

@rwjblue
Copy link
Collaborator

rwjblue commented Jun 3, 2021

Working on fixing this over in emberjs/ember.js#19586, that will reset the boostrapping back to an IIFE and restore functionality for Embroider 0.40 and 0.41 (without requiring changes in Embroider).

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Jun 3, 2021

Uncaught ReferenceError: Ember is not defined fix should have landed, I'll be moving on to the TypeError: block is not iterable issue as @rwjblue works on releasing the previous fix.

As part of this, I will be testing master of ember-source again master of embroider. Which should validate my previous claim... I'll report back

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Jun 3, 2021

Uncaught ReferenceError: Ember is not defined appears to have been fixed, but we have a new issue (on ember side). The iterator issue appears to not effect canary, and may be limited to older LTS versions of ember.

Current issue:

Build Error (PackagerRunner) in ../../../../../../emberjs/ember.js/@ember/-internals/runtime/type-tests/core.test.js

Module not found: Error: Can't resolve '../types/core' in '$TMPDIR/embroider/9caab5/emberjs/ember.js/@ember/-internals/runtime/type-tests/core.test.js'

This appears to not yet affect beta, but does effect canary, and we must fix it on embers side.

@stefanpenner
Copy link
Collaborator Author

Opened an ember issue, looking for a fix now: emberjs/ember.js#19588

@stefanpenner
Copy link
Collaborator Author

I have a fix for the ../types/core issue: emberjs/ember.js#19589

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Jun 3, 2021

TypeError: block is not iterable may be a heisenbug when running many test scenarios in sequence.

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Jun 3, 2021

Thinking about htis more TypeError: block is not iterable error could be a cash pollution issue when running in the following order:

  1. ember-lts-3.16 (pass)
  2. ember-lts-3.20 (fail)
  3. ember-lts-3.24 (fail)
  4. ember-release (surprisingly passes)

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Jun 3, 2021

A side note, we have spent a decent amount of time merely recovering ground here. Ember.js most likely needs to run 2 new CI smoke test jobs (one against classic ember app, and one against an embroider app).

I spoke with @rwjblue about this, and he is good with that approach. So most likely once we have the above remaining issue resolved, I hope to set up those tests. Having to regularly debug and diagnose these issues is a PITA, and such a smoke test on embers side has the potential of paying good dividends.

@stefanpenner
Copy link
Collaborator Author

I believe the TypeError: block is not iterable issue is actually a strange interaction between ember-cli-htmlbars-inline-precompile (which is not needed with ember-cli-htmlbars >=4.0./0) and those versions of ember. When removing the add-on, the issue appears to go away.

I'll do some additional testing, but I suspect we might be in the clear.

@stefanpenner
Copy link
Collaborator Author

I believe i have a solution to the TypeError: block is not iterable -> #837

@stefanpenner
Copy link
Collaborator Author

Unfortunately the problem is still around, I'll continue to debug

@stefanpenner
Copy link
Collaborator Author

removing $TMPDIR/embroider appears to correct the issue...

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Jun 7, 2021

The final issue appears to be embroiders lack of cache key creation when it comes to the webpack babel-loader. In this case specifically it appears to create a situation where babel -> inline hbs -> template-compiler's content is considered, thus allowing different versions of ember to pollute the cache.

We have resolve this issue some time ago in classic builds, now we must do similar here...

@stefanpenner
Copy link
Collaborator Author

After doing some spelunking, I believe an approach similar to (potentially even sharing the code with) classic builds is likely appropriate here. There are some trade-offs, and I'll keep those in mind as a I work on a POC.

@stefanpenner
Copy link
Collaborator Author

I have posted a PR which addresses my understanding of the last issue: #839

It also exposes two more currently undiscovered issues, related to the same problem, which I intend to follow up with fixes on next.

@rwjblue rwjblue linked a pull request Jun 9, 2021 that will close this issue
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 a pull request may close this issue.

2 participants