Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Publish to OpenVSX. #329

Merged
merged 2 commits into from
Mar 22, 2023
Merged

Publish to OpenVSX. #329

merged 2 commits into from
Mar 22, 2023

Conversation

rgrunber
Copy link
Member

I'll need to test this out a bit more thoroughly, but this should be the right approach.

@rgrunber rgrunber force-pushed the ovsx branch 3 times, most recently from 7c3f4f2 to 233ce06 Compare March 17, 2023 19:55
@codecov-commenter
Copy link

Codecov Report

Patch and project coverage have no change.

Comparison is base (485487b) 74.34% compared to head (233ce06) 74.34%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #329   +/-   ##
=======================================
  Coverage   74.34%   74.34%           
=======================================
  Files          90       90           
  Lines        3909     3909           
  Branches      689      689           
=======================================
  Hits         2906     2906           
  Misses       1003     1003           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rgrunber
Copy link
Member Author

rgrunber commented Mar 17, 2023

@lstocchi , in https://github.com/redhat-developer/vscode-knative/pull/322/files#diff-f6401d0dbc035410143f1adff95d0ae3a13fcb708414bd382f21bd54e5eabfb0 you removed some findNotifications calls relating to Downloading ${tool}. I needed to do the same for https://github.com/redhat-developer/vscode-knative/pull/329/files#diff-f6401d0dbc035410143f1adff95d0ae3a13fcb708414bd382f21bd54e5eabfb0R114 . It looks like findNotifications isn't able to detect the notification. Were you able to find the exact reason ?

Update: It looks like

await center.getDriver().sleep(1000);
is the culprit. Commenting out that line restores it again. I guess it makes sense given that the downloads didn't look to take longer than ~1s when running the UI tests.

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber rgrunber force-pushed the ovsx branch 2 times, most recently from e1f2f2c to e0223f1 Compare March 17, 2023 20:48
@lstocchi
Copy link
Contributor

@lstocchi , in https://github.com/redhat-developer/vscode-knative/pull/322/files#diff-f6401d0dbc035410143f1adff95d0ae3a13fcb708414bd382f21bd54e5eabfb0 you removed some findNotifications calls relating to Downloading ${tool}. I needed to do the same for https://github.com/redhat-developer/vscode-knative/pull/329/files#diff-f6401d0dbc035410143f1adff95d0ae3a13fcb708414bd382f21bd54e5eabfb0R114 . It looks like findNotifications isn't able to detect the notification. Were you able to find the exact reason ?

Update: It looks like

await center.getDriver().sleep(1000);

is the culprit. Commenting out that line restores it again. I guess it makes sense given that the downloads didn't look to take longer than ~1s when running the UI tests.

Hey Roland, sorry for the late response. Quite busy and i forgot to reply. AFAIK the ui tests in vscode knative have always been unstable. I know that Sudhir fought a bit on them before leaving. In that PR commenting that line did the magic but, if i recall correctly, the tests started failing again later. So to answer your question, no i didn't find the exact reason 😞

@rgrunber
Copy link
Member Author

Hey Roland, sorry for the late response. Quite busy and i forgot to reply. AFAIK the ui tests in vscode knative have always been unstable. I know that Sudhir fought a bit on them before leaving. In that PR commenting that line did the magic but, if i recall correctly, the tests started failing again later. So to answer your question, no i didn't find the exact reason disappointed

Thanks for the info. I guess commenting out the findNotifications line isn't so bad because ultimately we confirm in the line below it with safeNotification that it does exist. The only difference I saw between the 2 was that the latter doesn't have a 1s delay before checking notifications, and the downloads to happen quickly.

@mohitsuman , let me know if you can review . This should add ensure we publish to OpenVSX in the next release.

Copy link
Collaborator

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

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

LGTM

@rgrunber rgrunber merged commit 9f3c61d into redhat-developer:main Mar 22, 2023
@rgrunber rgrunber deleted the ovsx branch March 22, 2023 15:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish the vscode-knative extension to Open VSX
4 participants