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(resource): add if condition (DSP-1655) #448

Merged
merged 1 commit into from May 25, 2021

Conversation

kilchenmann
Copy link
Contributor

resolves DSP-1655

@kilchenmann kilchenmann self-assigned this May 21, 2021
@kilchenmann kilchenmann requested a review from mdelez May 21, 2021 13:26
@kilchenmann
Copy link
Contributor Author

@mdelez What do you think about this quick solution?

@mdelez
Copy link
Collaborator

mdelez commented May 21, 2021

In what scenario do you find yourself in a situation where a ValueAdded event is raised but there is no value?

@kilchenmann
Copy link
Contributor Author

kilchenmann commented May 21, 2021

In what scenario do you find yourself in a situation where a ValueAdded event is raised but there is no value?

only in the test... yes maybe we should fix it in the test. But I have no clue about this setup. The test failed because "newValue" is undefined: TypeError: Cannot read property 'addedValue' of undefined

@kilchenmann
Copy link
Contributor Author

@mdelez Maybe I'll need your help here. Any suggestion?

@mdelez
Copy link
Collaborator

mdelez commented May 25, 2021

@mdelez Maybe I'll need your help here. Any suggestion?

I can take a look at it later today

@kilchenmann
Copy link
Contributor Author

Maybe @flavens has also an idea!?

@mdelez
Copy link
Collaborator

mdelez commented May 25, 2021

Strangely, if I run npm run test-ci locally, I get a different error message. I can't get the same ValueAdded error 🤔 What happens when you run this command locally? No errors?

Screen Shot 2021-05-25 at 11 47 11

@kilchenmann
Copy link
Contributor Author

Strangely, if I run npm run test-ci locally, I get a different error message. I can't get the same ValueAdded error 🤔 What happens when you run this command locally? No errors?

Yes, this is the strange thing. I do not have an error, but sometimes I have the same error as on Github CI. But sometimes I also had your error message, but test were successful. This is what happened right now on main branch...

@kilchenmann
Copy link
Contributor Author

On Github CI the tests failed on the mentioned error Cannot read property 'addedValue' of undefined, but when re-running all the tests, they are successful. But this could not be the solution, right?

@mdelez
Copy link
Collaborator

mdelez commented May 25, 2021

On Github CI the tests failed on the mentioned error Cannot read property 'addedValue' of undefined, but when re-running all the tests, they are successful. But this could not be the solution, right?

This is definitely a solution but I feel like we should figure out why this is actually happening at some point. My guess is that it's a timing issue. There is probably an async BeforeEach somewhere that is not ready in time sometimes. For now, I'll approve it.

@kilchenmann
Copy link
Contributor Author

On Github CI the tests failed on the mentioned error Cannot read property 'addedValue' of undefined, but when re-running all the tests, they are successful. But this could not be the solution, right?

This is definitely a solution but I feel like we should figure out why this is actually happening at some point. My guess is that it's a timing issue. There is probably an async BeforeEach somewhere that is not ready in time sometimes. For now, I'll approve it.

Yes absolutely, I agree.

@kilchenmann kilchenmann merged commit 656da04 into main May 25, 2021
@kilchenmann kilchenmann deleted the wip/dsp-1655-fix-unit-tests branch May 25, 2021 11:53
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.

None yet

2 participants