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

:arrow_up: tree-view@0.224.0 #17088

Merged
merged 4 commits into from Aug 21, 2018

Conversation

Projects
None yet
5 participants
@Ben3eeE
Member

Ben3eeE commented Apr 5, 2018

Description of the change

Atom 1.25 and 1.26 ships with tree-view@0.221.3. This is an older version that has not been upgraded since the tests were not passing. This failing test has been fixed in tree-view@0.223.0. After this was merged tree-view was bumped to 0.222.0 instead of 0.223.0. This seemed like a mistake and that the intent was to upgrade to tree-view@0.223.0. I asked about this and @50Wliu confirmed that it seems like we should be on tree-view@0.223.0. I tried upgrading tree-view directly on the master branch but there seems to be a failing test on circleci after upgrading.

Comparing the current shipped version and the latest version:
atom/tree-view@v0.221.3...v0.223.0

Comparing the current version on atom/atom master branch with the latest version:
atom/tree-view@v0.222.0...v0.223.0

/cc: @50Wliu @daviwil

@daviwil

daviwil approved these changes Apr 5, 2018

Thanks a lot Linus!

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Apr 6, 2018

Member

Updated the description so it's clear why this was opened as a PR.

Member

Ben3eeE commented Apr 6, 2018

Updated the description so it's clear why this was opened as a PR.

@daviwil

This comment has been minimized.

Show comment
Hide comment
@daviwil

daviwil Apr 12, 2018

Member

I've only been able to replicate this test failure on macOS, not sure what the difference is there. Will look into it some more tomorrow.

Member

daviwil commented Apr 12, 2018

I've only been able to replicate this test failure on macOS, not sure what the difference is there. Will look into it some more tomorrow.

@UziTech

This comment has been minimized.

Show comment
Hide comment
@UziTech

UziTech Jul 31, 2018

Contributor

Any update on this?

Atom v1.29 still has tree-view@0.222.0

Contributor

UziTech commented Jul 31, 2018

Any update on this?

Atom v1.29 still has tree-view@0.222.0

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Aug 10, 2018

Member

@UziTech I'll ping the team about updates. For now I upgraded to 0.224.0 and restarted the builds with VSTS.

@50Wliu FYI: I changed this PR to 0.224.0. You did all the commits between 0.223.0 and 0.224.0 🙂

Member

Ben3eeE commented Aug 10, 2018

@UziTech I'll ping the team about updates. For now I upgraded to 0.224.0 and restarted the builds with VSTS.

@50Wliu FYI: I changed this PR to 0.224.0. You did all the commits between 0.223.0 and 0.224.0 🙂

@Ben3eeE Ben3eeE changed the title from :arrow_up: tree-view@0.223.0 to :arrow_up: tree-view@0.224.0 Aug 10, 2018

@maxbrunsfeld maxbrunsfeld self-assigned this Aug 20, 2018

Ben3eeE and others added some commits Apr 5, 2018

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Aug 20, 2018

Contributor

Ok, made some progress on this. The failures are due to atom/tree-view#1048. When the tree-view tests are run as part of Atom's CI, the project folder is set to atom/node_modules/tree-view/spec/fixtures, which matches Atom's .gitignore because it's inside of node_modules.

I think I need to adjust the failing test to use a temp directory as its project folder so that paths will not be considered as ignored.

Contributor

maxbrunsfeld commented Aug 20, 2018

Ok, made some progress on this. The failures are due to atom/tree-view#1048. When the tree-view tests are run as part of Atom's CI, the project folder is set to atom/node_modules/tree-view/spec/fixtures, which matches Atom's .gitignore because it's inside of node_modules.

I think I need to adjust the failing test to use a temp directory as its project folder so that paths will not be considered as ignored.

maxbrunsfeld added a commit to atom/tree-view that referenced this pull request Aug 20, 2018

Avoid using the project directory itself as an example for tests
The tree-view directory itself is ignored when these tests are run as
part of Atom's CI.

Refs #1048
Refs atom/atom#17088
@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Aug 21, 2018

Contributor

The latest failure is a keybinding-resolver failure which looks like a flake.

2018-08-21T00:24:36.2759500Z KeyBindingResolverView
2018-08-21T00:24:36.2774560Z   when a keydown event occurs
2018-08-21T00:24:36.2792500Z     it displays all commands for the keydown event but does not clear for the keyup when there is no keyup binding
2018-08-21T00:24:36.2810360Z       TypeError: Cannot read property 'textContent' of null
2018-08-21T00:24:36.2829920Z         at it (/Users/vsts/agent/2.136.1/work/1/s/node_modules/keybinding-resolver/spec/keybinding-resolver-view-spec.js:49:80)
2018-08-21T00:24:36.2853890Z         at <anonymous>
2018-08-21T00:24:36.2861710Z 
2018-08-21T00:24:36.2869680Z 
2018-08-21T00:24:36.2886220Z Finished in 0.205 seconds
2018-08-21T00:24:36.2902710Z 3 tests, 5 assertions, 1 failure, 0 skipped
Contributor

maxbrunsfeld commented Aug 21, 2018

The latest failure is a keybinding-resolver failure which looks like a flake.

2018-08-21T00:24:36.2759500Z KeyBindingResolverView
2018-08-21T00:24:36.2774560Z   when a keydown event occurs
2018-08-21T00:24:36.2792500Z     it displays all commands for the keydown event but does not clear for the keyup when there is no keyup binding
2018-08-21T00:24:36.2810360Z       TypeError: Cannot read property 'textContent' of null
2018-08-21T00:24:36.2829920Z         at it (/Users/vsts/agent/2.136.1/work/1/s/node_modules/keybinding-resolver/spec/keybinding-resolver-view-spec.js:49:80)
2018-08-21T00:24:36.2853890Z         at <anonymous>
2018-08-21T00:24:36.2861710Z 
2018-08-21T00:24:36.2869680Z 
2018-08-21T00:24:36.2886220Z Finished in 0.205 seconds
2018-08-21T00:24:36.2902710Z 3 tests, 5 assertions, 1 failure, 0 skipped

@maxbrunsfeld maxbrunsfeld merged commit 1a2dbb6 into master Aug 21, 2018

2 of 3 checks passed

VSTS: Atom Pull Requests 20180820.16 failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maxbrunsfeld maxbrunsfeld deleted the b3-bump-tree-view branch Aug 21, 2018

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Aug 21, 2018

Member

Thanks a ton @maxbrunsfeld! Will be great to have all the new tree-view improvements.

Member

50Wliu commented Aug 21, 2018

Thanks a ton @maxbrunsfeld! Will be great to have all the new tree-view improvements.

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Aug 21, 2018

Member

Thanks @maxbrunsfeld 🎊 Excited to see all the new tree-view improvements get released.

Member

Ben3eeE commented Aug 21, 2018

Thanks @maxbrunsfeld 🎊 Excited to see all the new tree-view improvements get released.

@jasonrudolph jasonrudolph referenced this pull request Aug 23, 2018

Closed

Iteration Plan: August 20 - August 31 #17910

13 of 16 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment