FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Test static route by code-asher · Pull Request #3297 · coder/code-server · GitHub

Test static route - #3297

Merged
code-asher merged 6 commits into
coder:mainfrom
code-asher:test-static
May 6, 2021
Merged

Test static route#3297
code-asher merged 6 commits into
coder:mainfrom
code-asher:test-static

Conversation

code-asher commented May 5, 2021
edited
Loading

Copy link
Copy Markdown
Member

Mostly just wanted to add a test for static auth.

codecov Bot commented May 5, 2021
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #3297 (e8443e2) into main (3243bb3) will increase coverage by 1.99%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3297      +/-   ##
==========================================
+ Coverage   56.95%   58.95%   +1.99%     
==========================================
  Files          35       35              
  Lines        1703     1703              
  Branches      374      374              
==========================================
+ Hits          970     1004      +34     
+ Misses        583      561      -22     
+ Partials      150      138      -12     
Impacted Files Coverage Δ
src/node/util.ts 50.00% <0.00%> (+1.02%) ⬆️
src/node/cli.ts 79.57% <0.00%> (+1.70%) ⬆️
src/node/main.ts 40.22% <0.00%> (+3.44%) ⬆️
src/node/http.ts 32.78% <0.00%> (+4.91%) ⬆️
src/node/routes/index.ts 77.14% <0.00%> (+9.52%) ⬆️
src/node/routes/static.ts 60.46% <0.00%> (+30.23%) ⬆️

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 3243bb3...e8443e2. Read the comment docs.

code-asher marked this pull request as ready for review May 5, 2021 23:05
code-asher requested a review from a team as a code owner May 5, 2021 23:05
jsjoeio added the testing Anything related to testing label May 5, 2021
jsjoeio added this to the v3.9.4 milestone May 5, 2021

jsjoeio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks great! 🚀

Mostly nits around test names and then a question

Comment thread test/unit/helpers.test.ts
Comment thread test/unit/routes/static.test.ts Outdated
Comment thread test/unit/routes/static.test.ts Outdated
Comment thread test/unit/routes/static.test.ts Outdated
Comment thread test/unit/routes/static.test.ts Outdated
code-asher force-pushed the test-static branch 6 times, most recently from b44ba1e to e382328 Compare May 6, 2021 16:29
code-asher marked this pull request as draft May 6, 2021 16:52

code-asher commented May 6, 2021
edited
Loading

Copy link
Copy Markdown
Member Author

I need to refactor the logger module mock because it causes errors in the e2e tests (now that the terminal test imports the helper file) but that conflicts with some constant test changes you're making so I'm going to wait for that to get merged then rebase.

jsjoeio commented May 6, 2021

Copy link
Copy Markdown
Contributor

I'm going to wait for that to get merged then rebase.

code-asher added 5 commits May 6, 2021 14:25
This is to match the other tests that create temp directories. It also
lets you clean up test temp directories all at once separately from
other non-test temporary directories.
Just to limit all the noise from code-server's startup output.
code-asher force-pushed the test-static branch 2 times, most recently from 01f695f to eb34f4b Compare May 6, 2021 19:47
code-asher marked this pull request as ready for review May 6, 2021 20:01
code-asher requested a review from jsjoeio May 6, 2021 20:01
It errors that jest is not defined so put it behind a function instead
of immediately creating the mock (this is probably a better pattern
anyway).

The constant tests had to be reworked a little. Since the logger mock is
hoisted it runs before createLoggerMock is imported. I moved it into a
beforeAll which means the require call also needed to be moved
there (since we need to mock the logger before requiring the constants
or it'll pull the non-mocked logger).

This means getPackageJson needs to be a let and assigned afterward. To
avoid having to define a type for getPackageJson I just added a let var
set to the type of the imported constants file and modified the other
areas to use the same paradigm.

I also replaced some hardcoded strings with the mocked package.json
object.

jsjoeio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nice work!

code-asher merged commit 4f320ad into coder:main May 6, 2021
code-asher deleted the test-static branch May 6, 2021 23:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Anything related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL