Skip to content

Implement last opened functionality #4633

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
Merged
merged 14 commits into from
Dec 17, 2021
Merged

Implement last opened functionality #4633

merged 14 commits into from
Dec 17, 2021

Conversation

code-asher
Copy link
Member

@code-asher code-asher commented Dec 16, 2021 •
edited
Loading

Fixes #4619

This also restores the settings query file write which we were missing.

@code-asher code-asher requested a review from a team as a code owner December 16, 2021 00:10
@code-asher code-asher force-pushed the last-opened branch 3 times, most recently from 7b27250 to 92865f2 Compare December 16, 2021 00:15
@codecov
Copy link

codecov bot commented Dec 16, 2021 •
edited
Loading

Codecov Report

Merging #4633 (7b27250) into main (73e0b79) will decrease coverage by 0.41%.
The diff coverage is 8.33%.

❗ Current head 7b27250 differs from pull request most recent head 92865f2. Consider uploading reports for the commit 92865f2 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4633      +/-   ##
==========================================
- Coverage   66.10%   65.68%   -0.42%     
==========================================
  Files          30       30              
  Lines        1658     1670      +12     
  Branches      337      343       +6     
==========================================
+ Hits         1096     1097       +1     
- Misses        477      486       +9     
- Partials       85       87       +2
Impacted Files Coverage Δ
src/node/routes/vscode.ts 40.32% <8.33%> (-7.68%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73e0b79...92865f2. Read the comment docs.

jsjoeio
jsjoeio approved these changes Dec 16, 2021
View reviewed changes
@jsjoeio jsjoeio requested a review from a team December 16, 2021 23:38
@code-asher
Implement last opened functionality
f482c2a 
Fixes coder#4619
@code-asher
Fix test temp dirs not being cleaned up
9a8477b
@code-asher
Mock logger everywhere
902a820 
This suppresses all the error and debug output we generate which makes
it hard to actually find which test has failed.  It also gives us a
standard way to test logging for the few places we do that.
@code-asher
Use separate data directories for unit test instances
2fbb43f 
Exactly as we do for the e2e tests.
@code-asher
fixup! Implement last opened functionality
08cc53f
@code-asher
Add integration tests for vscode route
953f3a4
@code-asher
Make settings use --user-data-dir
6860c7b 
Without this test instances step on each other feet and they also
clobber your own non-test settings.
@code-asher
Copy link
Member Author

code-asher commented Dec 17, 2021 •
edited
Loading

Testing this took a bit of doing since I wanted to use the integration tests as it uses both the req object and redirections.

It was hard to see test failures among all the logger output so I added a logger mock and used that everywhere to get rid of the noise.

As I was doing that I noticed some accumulating test dirs so I took care of those.

I had issues with the settings from the tests stepping all over each other and conflicting with my own running instance of code-server so I had to add some separation to the instances which meant making the settings class use the path provided on the command line.

I also added some tests to make sure relative paths are being generated correctly.

@code-asher code-asher requested a review from jsjoeio December 17, 2021 00:29
@code-asher
Remove compilation check
2a08ee5 
If you do a regular non-watch build there are no compilation stats so
this bricks VS Code in CI when running the unit tests.

I am not sure how best to fix this for the case where you have a build
that has not been packaged yet so I just removed it for now and added a
message to check if VS Code is compiling when in dev mode.
jsjoeio
jsjoeio approved these changes Dec 17, 2021
View reviewed changes
Copy link
Contributor

@jsjoeio jsjoeio left a comment

Choose a reason for hiding this comment

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

LGTM! Nice cleanup 👏

Approving but make sure you look at the URL provided to the Updater. I think it should use the new GitHub org

@code-asher code-asher merged commit c4c480a into coder:main Dec 17, 2021
@code-asher code-asher deleted the last-opened branch December 17, 2021 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsjoeio jsjoeio jsjoeio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Close folder does not appear to work
2 participants
@code-asher @jsjoeio