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.

Sign up for GitHub

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

Jump to bottom

[BUG] More broken fix faulty tests #4884

Closed
Felienne opened this issue Dec 11, 2023 · 4 comments · Fixed by #4881
Closed

[BUG] More broken fix faulty tests #4884

Felienne opened this issue Dec 11, 2023 · 4 comments · Fixed by #4881
Assignees
@ToniSkulj

Comments

@Felienne
Copy link
Member

Hi @ToniSkulj!

I am afraid I am wrecking more havoc on your tests, sorry! See: #4881

I commented out the broken tests for now. Could you take a look 🙏?

(I suspect that #4879 too might have something to do with the skip faulty code, I took a look at what is going wrong but it does seem to raise the right exception, it looks like it is just caught somewhere in your code)
@ToniSkulj
Copy link
Member

Hi @Felienne!

No worries!! I will take a look at the broken tests tonight.

@Felienne
Copy link
Member Author

Thanks!!

@Felienne
Copy link
Member Author

Felienne commented Dec 11, 2023 •
edited
Loading

Hi @ToniSkulj

I suspect that #4887 fixes this too, but I did not have time to check! So before you dive in and spend time, you might want to explore that haha

@ToniSkulj
Copy link
Member

Hi @Felienne!

You were right! #4887 did fix the issues with the tests.
But other tests started failing 😅. Luckily, I found the problem and fixed it with commit after merging #4887 into #4881

everything should work now (and is ready for deployment?)
Therefore, the PR (4881) also fixes #4884 now.

@Felienne Felienne mentioned this issue Dec 12, 2023
Merged
@mergify mergify bot closed this as completed in #4881 Dec 13, 2023
mergify bot pushed a commit that referenced this issue Dec 13, 2023
@Felienne
[LANGUAGE] Warning unused var (#4881)
fe822d1 
Fixes #2394 (finally!!) by checking if declared variables are actually used, also fixes #4884 that was introduced here.

Depends-On: #4880

**How to test**

I have added a test for the exception. To see it in action on the front-end, run code that defines but not uses a variable, and observe you get a warning but the code still runs:

<img width="1382" alt="image" src="https://github.com/hedyorg/hedy/assets/1003685/bb00c145-8d81-4e86-8ea1-316b0ae955c6">

Note that this PR needs #4880 to show the warning in the front-end!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ToniSkulj ToniSkulj

Labels
None yet
Projects
Hedy organization board
Status: No status
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[LANGUAGE] Warning unused var hedyorg/hedy
2 participants
@Felienne @ToniSkulj