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

test: add potential failing test for #110 #115

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Techn1x
Copy link
Contributor

@Techn1x Techn1x commented Sep 13, 2024

aimed to be a failing test for #110

Copy link

vercel bot commented Sep 13, 2024

@Techn1x is attempting to deploy a commit to the universal-ember Team on Vercel.

A member of the Team first needs to authorize it.

@Techn1x Techn1x marked this pull request as draft September 13, 2024 08:01
Comment on lines 276 to 279
<template>
<div {{logText this.testCase.endResult}} />
<out>{{this.testCase.endResult}}</out>
<button type="button" {{on "click" this.setTestCase}}></button>
Copy link
Contributor Author

@Techn1x Techn1x Sep 16, 2024

Choose a reason for hiding this comment

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

The secret sauce to get the test to fail was using the value in a modifier before using it for something else.

This test passes when either;

  • you comment out the div with the modifier, or
  • the value is referenced before the modifier (eg adding {{log this.testCase.endResult}} to the line before the div, or
  • you use < ember 5.5ember 5.6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test passes when either;

  • or, my production builds seem to behave OK. I can't verify this here in the test though.

@NullVoxPopuli
Copy link
Contributor

so I pulled this down, only to realize that it needs rebased!

can you rebase? thanks!!
(and thanks for the repro!)

@Techn1x Techn1x force-pushed the potential-failing-test branch from ce5469d to 5525e17 Compare September 17, 2024 00:45
@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 17, 2024

Whoops! I was operating out of an old fork.

Rebased.

Feel free to modify / undraft this work how you see fit. I called the test "failing case" for lack of a better name, given I'm not sure what the core problem is that it's testing for

@NullVoxPopuli
Copy link
Contributor

image

image

5.5 is ok, but 5.6 isn't -- this matches my expectations of ember-source, because 5.6 had the VM update

@NullVoxPopuli
Copy link
Contributor

I'm still investigating, but my worry is that this is related to either of these two PRs:

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 18, 2024

Would either of those PRs be expected to exhibit different behaviour between test / prod builds?

@NullVoxPopuli
Copy link
Contributor

I wouldn't expect a difference in behavior, no -- but that doesn't mean something accidentally didn't happen! 🙈

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