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

feat: rename x-deno- to x-nf- #5398

Merged
merged 3 commits into from
Jan 17, 2023
Merged

feat: rename x-deno- to x-nf- #5398

merged 3 commits into from
Jan 17, 2023

Conversation

Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Jan 17, 2023

Part of https://github.com/netlify/partner-deno/issues/11. Renames the x-deno- headers to x-nf-. Will fail right now, but work once netlify/edge-bundler#290 lands.

@Skn0tt Skn0tt self-assigned this Jan 17, 2023
@Skn0tt Skn0tt added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jan 17, 2023
@github-actions
Copy link

github-actions bot commented Jan 17, 2023

📊 Benchmark results

Comparing with 6726bf4

Package size: 267 MB

⬇️ 0.00% decrease vs. 6726bf4

^                  267 MB  267 MB 
│  260 MB  260 MB   ┌──┐    ┌──┐  
│   ┌──┐    ┌──┐    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴──>
    T-3     T-2     T-1      T    
Legend

@Skn0tt
Copy link
Member Author

Skn0tt commented Jan 17, 2023

ab1da2c should've failed some tests 🤔 why is it not?

@Skn0tt
Copy link
Member Author

Skn0tt commented Jan 17, 2023

why is it not?

it should've failed tests in https://github.com/netlify/cli/blob/main/tests/integration/100.command.dev.test.cjs, but these tests were never executed. It looks like that's because we use Vitest's --changed option, which only executes tests that are dependent on the changed file. That dependency is detected by tracing imports, but since integration tests don't import the source code but run the CLI via execa instead, it can't detect how these fit together.

Here's the imports for the test file that should've been run, but wasn't:

// Handlers are meant to be async outside tests
const path = require('path')
// eslint-disable-next-line ava/use-test
const avaTest = require('ava')
const { isCI } = require('ci-info')
const dotProp = require('dot-prop')
const jwt = require('jsonwebtoken')
const { Response } = require('node-fetch')
const { withDevServer } = require('./utils/dev-server.cjs')
const got = require('./utils/got.cjs')
const { withMockApi } = require('./utils/mock-api.cjs')
const { pause } = require('./utils/pause.cjs')
const { withSiteBuilder } = require('./utils/site-builder.cjs')
const test = isCI ? avaTest.serial.bind(avaTest) : avaTest
const JWT_EXPIRY = 1_893_456_000
const getToken = ({ jwtRolePath = 'app_metadata.authorization.roles', jwtSecret = 'secret', roles }) => {

As you can see, there's no import of the source files in there, so Vitest's --change dependency tracing doesn't pick it up.

I'll open a separate issue to track this bug in our CI. Let's go ahead with this PR.

@Skn0tt Skn0tt marked this pull request as ready for review January 17, 2023 13:51
@Skn0tt Skn0tt requested a review from a team January 17, 2023 13:51
@Skn0tt Skn0tt added the automerge Add to Kodiak auto merge queue label Jan 17, 2023
@kodiakhq kodiakhq bot merged commit 87adddc into main Jan 17, 2023
@kodiakhq kodiakhq bot deleted the rename-x-deno-x-nf branch January 17, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Add to Kodiak auto merge queue type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants