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] Highlighted commands are shown in errors again #4468

Merged
merged 7 commits into from
Sep 12, 2023

Conversation

jpelay
Copy link
Member

@jpelay jpelay commented Sep 8, 2023

Description

There's a bug right now in Hedy were the keywords in the errors are not being highlighted:

image

This is due to a syntax error in additional.css. Now is back to normal:

image

@Felienne
Copy link
Member

Felienne commented Sep 8, 2023

Haha I just noticed this while working on something and I thought I had broken it :) Thanks for the fix!

@jpelay
Copy link
Member Author

jpelay commented Sep 8, 2023

Haha I just noticed this while working on something and I thought I had broken it :) Thanks for the fix!

hahaha No worries, I'm trying to get the tests to work and then it'll be ready!

@jpelay
Copy link
Member Author

jpelay commented Sep 8, 2023

The test were failing in the command line because of timing. I looked up for a while, but couldn't get it to work without the wait. I left a TODO comment to tackle this in the future.

@Felienne
Copy link
Member

Felienne commented Sep 9, 2023

Let's see if a higher time works!

@jpelay
Copy link
Member Author

jpelay commented Sep 9, 2023

Let's see if a higher time works!

Yes let's see! Hopefully it will! For me it also worked without waits in the command line using the option --browser chrome but I'm not sure if that's feasible on Github Actions

@Felienne
Copy link
Member

Felienne commented Sep 9, 2023

Let's see if a higher time works!

Yes let's see! Hopefully it will! For me it also worked without waits in the command line using the option --browser chrome but I'm not sure if that's feasible on Github Actions

I don't know either, so I am not sure how we can move this forward at the moment

@jpelay
Copy link
Member Author

jpelay commented Sep 9, 2023

I don't know either, so I am not sure how we can move this forward at the moment

According to Cypress, yes we can, but since I'm working in the debugger at the moment, I'll tackle this on Monday! Do I set this as a draft in the meantime?

@jpelay
Copy link
Member Author

jpelay commented Sep 11, 2023

The test that was failing was a flaky cypress test that I already opened an issue for in #4455.

Copy link
Member

@Felienne Felienne left a comment

Choose a reason for hiding this comment

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

Yes, works again and now with passing tests!

image

Will also deploy this now cause it is a bit annoying that is is broken.

@mergify
Copy link
Contributor

mergify bot commented Sep 12, 2023

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 8b8be5d into hedyorg:main Sep 12, 2023
8 checks passed
@mergify
Copy link
Contributor

mergify bot commented Sep 12, 2023

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@jpelay
Copy link
Member Author

jpelay commented Sep 12, 2023

Sigh, these test failed in https://github.com/hedyorg/hedy/actions/runs/6158212111 :c in #4471

@Felienne
Copy link
Member

Sigh, these test failed in https://github.com/hedyorg/hedy/actions/runs/6158212111 :c in #4471

Let's not worry about it all too much, we can discuss tonight!

mergify bot pushed a commit that referenced this pull request Sep 13, 2023
In #4468 we merged a few cypress tests that had to do with the editor; they executed correctly in that PR, but after that they started failing consistently. So, in this PR I'm removing them from the test suite, leaving a comment stating that we need to come back to it, and also noting it in #4455.
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