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

Acceptance tests started failing after upgrading from 1.15.4 to 1.16.0 #495

Closed
vsergiu93 opened this issue Mar 31, 2020 · 8 comments
Closed
Labels

Comments

@vsergiu93
Copy link

I can't get my acceptance tests to work after upgrading from 1.15.4 to 1.16.0, I think it's important to say only acceptance tests are affected, integration tests that uses page objects are passing.

Relevant package.json entries:

{
  "devDependencies": {
     ...
      "ember-qunit": "^4.6.0",
      "ember-source": "~3.17.2",
      "ember-cli-page-object": "1.16.0",
     ...
  }
}

Console errror:
image

@ro0gr
Copy link
Collaborator

ro0gr commented Mar 31, 2020

Thanks for reporting!

Is ember-cli-page-object the only package which you do upgrade?

From the error message, it looks like the issue may be related to updating some ember related package, cause lookup is not an ember-cli-page-object API.

If that's not the case yarn list @ember/test-helpers or npm list @ember/test-helpers output could be also helpful.

@vsergiu93
Copy link
Author

@ro0gr I'm only updating the ember-cli-page-object package, I mean this is a dependabot PR which I try to fix before merging.

Also, not sure if I was clear in my issue description, but, only the acceptance tests fail and also only those that rely on page object, integration tests work fine and all are green.

Here is the output of (yarn list @ember/test-helpers):
└─ @ember/test-helpers@1.7.1

@ro0gr
Copy link
Collaborator

ro0gr commented Apr 1, 2020

ok, thanks. Let's try to find that regression then.

From the stacktrace I still can't find any clue related to page objects code.

'lookup' of undefined

looks like something tries to use Ember DI APIs on a missing variable. Does it fail on attempt visit? Also, you say "Acceptance", does it mean "moduleForAcceptance" or the RFC268 setupApplicationTest?

I've just checked and Acceptance/Application tests are green for 1.16. If you have a chance to create a reduced reproduction, it would be great and speed up the process of finding the cause a lot!

@vsergiu93
Copy link
Author

I'm using RFC268. I'll try to reproduce it on a fresh app,
Thank you for your time.

@vsergiu93
Copy link
Author

@ro0gr After some further testing it seems the code that started to affect my tests was introduced in #448. More exactly the changes to getExecutionContext function, even though we use RFC268 style tests, somehow the contextName gets assigned the value acceptance after the if/else, but it should be rfc268.

More context

The app I'm working on has been initially started on ember 2.16 and last summer we spent almost a month upgrading it to 3.13, it is possible that during the upgrade process we skipped something important, and because of that getExecutionContext isn't able to properly determine correctly what style of acceptance test we have.
I looked at our test/test-helpers.js and I see we have this line App.injectTestHelpers() and after commenting that line the 'lookup' of undefined error is not occurring anymore.

Conclusion

I guess we can close this issue since it seems more of problem in our project than a problem with the package itself.

Thank your for your time

@ro0gr
Copy link
Collaborator

ro0gr commented Apr 2, 2020

hmm.. sounds it like it makes it impossible to have a test suite mixed of Acceptance and Application tests, if that test suite does injectTestHelpers().

Thank you for figuring out the root cause!

@ro0gr ro0gr added the bug label Apr 2, 2020
ro0gr added a commit that referenced this issue Apr 4, 2020
ro0gr added a commit to ro0gr/ember-cli-page-object that referenced this issue Apr 19, 2020
@ro0gr
Copy link
Collaborator

ro0gr commented Apr 19, 2020

@vsergiu93 Though seems the issue doesn't affect you anymore, I think it still made sense to fix it. Just published a fix as v1.16.3. It would be also included to the v1.17.2. Thanks again for finding it!

@ro0gr ro0gr closed this as completed Apr 19, 2020
@vsergiu93
Copy link
Author

@ro0gr Thank you, sorry I had other issues I had to work on and I forgot to follow the progress of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants