Skip to content

Commit

Permalink
Merge branch 'master' into patch-1
Browse files Browse the repository at this point in the history
  • Loading branch information
omercnet authored Mar 24, 2024
2 parents 3ddf6d4 + 96f3223 commit de2cabe
Show file tree
Hide file tree
Showing 18 changed files with 440 additions and 90 deletions.
18 changes: 9 additions & 9 deletions .github/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,18 @@ changelog:
labels:
- release # Exclude release PRs from changelog
categories:
- title: datadog-ci
labels:
- '*'
- title: Dependencies
labels:
- dependencies
- dependencies # Generally about vulnerability fixes
- title: Documentation
labels:
- documentation
- title: Chores
labels:
- chores
- title: CI Visibility
labels:
- ci-visibility
- title: Static Analysis
labels:
- static-analysis
- title: RUM
labels:
- rum
Expand All @@ -31,9 +28,12 @@ changelog:
- title: Source Code Integration
labels:
- source-code-integration
- title: Static Analysis
labels:
- static-analysis
- title: Synthetics
labels:
- synthetics
- title: datadog-ci
- title: Chores
labels:
- '*'
- chores
12 changes: 7 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,17 @@ jobs:
- run: mkdir artifacts
- run: yarn pack --filename artifacts/datadog-ci-${{ matrix.version }}.tgz
- run: cp -r .github/workflows/e2e artifacts/
- uses: actions/upload-artifact@v1
- uses: actions/upload-artifact@v4
with:
name: artifacts
name: artifacts-${{ matrix.version }}
path: artifacts/
- uses: actions/upload-artifact@v3
- uses: actions/upload-artifact@v4
if: always()
with:
name: sarif-datadog-ci.sarif
path: sarif-datadog-ci.sarif
if-no-files-found: error
overwrite: true

e2e-test:
strategy:
Expand All @@ -74,9 +75,10 @@ jobs:
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.version }}
- uses: actions/download-artifact@v1
- uses: actions/download-artifact@v4
with:
name: artifacts
name: artifacts-${{ matrix.version }}
path: artifacts/
- run: yarn add ./artifacts/datadog-ci-${{ matrix.version }}.tgz
- name: Run synthetics test
run: yarn datadog-ci synthetics run-tests --config artifacts/e2e/global.config.json
Expand Down
21 changes: 4 additions & 17 deletions .github/workflows/publish-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,37 +17,24 @@ jobs:
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const { data: releaseNotes } = await github.rest.repos.generateReleaseNotes({
owner: context.repo.owner,
repo: context.repo.repo,
tag_name: '${{ github.ref_name }}',
})
const { data: release } = await github.rest.repos.createRelease({
owner: context.repo.owner,
repo: context.repo.repo,
tag_name: '${{ github.ref_name }}',
name: '${{ github.ref_name }}',
draft: true,
prerelease: false,
body: releaseNotes.body,
generate_release_notes: true,
})
console.debug('Release notes:', JSON.stringify(releaseNotes.body))
// https://github.com/actions/toolkit/issues/1689
const releaseNotesToCopy = releaseNotes.body.replace('-->\n\n', '-->\n')
const editReleaseLink = release.html_url.replace('datadog-ci/releases/tag/', 'datadog-ci/releases/edit/')
await core.summary
.addHeading('Draft release created')
.addRaw('Find it here: ')
.addLink('${{ github.ref_name }} (draft)', release.html_url)
.addBreak()
.addBreak()
.addRaw('Copy the release notes below, and paste them inside your release PR.')
.addRaw('Please go to the link below, copy the release notes and paste them in your release PR.', true)
.addBreak()
.addBreak()
.addCodeBlock(releaseNotesToCopy, 'markdown')
.addLink('Edit ${{ github.ref_name }} (draft)', editReleaseLink)
.write()
return release.id
Expand Down
35 changes: 30 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ yarn launch synthetics run-tests --config dev/global.config.json
- [jest](https://github.com/facebook/jest): Tests are written in Jest.
- [volta](https://github.com/volta-cli/volta): NodeJS and yarn versioning.

### Repository structure
### Creating a new command

Follow the [Structure](#structure) below for any commands you add. Then, don't forget the [Things to update](#things-to-update) in the project.

#### Structure

Commands are stored in the [src/commands](src/commands) folder.

Expand All @@ -34,8 +38,6 @@ src/
└── index.ts
```

Documentation of the command must be placed in the README.md file, the [current README](/README.md) must be updated to link to the new command README.

The `index.ts` file must export classes extending the `Command` class of `clipanion`. The commands of all `src/commands/*/index.ts` files will then be imported and made available in the `datadog-ci` tool.

A sample `index.ts` file for a new command would be:
Expand All @@ -52,9 +54,32 @@ export class HelloWorldCommand extends Command {
module.exports = [HelloWorldCommand]
```

Lastly, test files must be created in the `__tests__/` folder. `jest` is used to run the tests and a CI has been set using GitHub Actions to ensure all tests are passing when merging a Pull Request.
Lastly, unit tests must be created in the `__tests__/` folder. The tests can then be launched with the `yarn test` command: it finds all files with a filename ending in `.test.ts` in the repo and executes them.

#### Beta command

If your command is related to a beta product or feature, or you want to test out the command first, you can mark your command as beta.

To do so, add your command's name to the [`BETA_COMMANDS` array](https://github.com/DataDog/datadog-ci/blob/35c54e1d1e991d21461084ef2e346ca1c6bb7ea6/src/cli.ts#L8).

Users have to prefix their command line with `DD_BETA_COMMANDS_ENABLED=1` to use the command. Make sure to document this in your command's README for visibility. This should be removed once the command goes out of beta.

Optionally, you can create a pre-release for your command by following the [Pre-Release Process](#pre-release-process) instructions below.

#### Things to update

- The [Usage section in the root README](README.md#usage) must be updated to link to:
- The new command's README.
- And 📚 should link to the official Datadog documentation site.
- **Note:** If your command is beta, use the [Beta commands](README.md#beta-commands) section instead.

- The command should be added under the right product in the [CODEOWNERS](.github/CODEOWNERS) file to ensure the right people are notified when a PR is opened.
- If you are only adding a sub-command (e.g. `datadog-ci <existing-command> <new-sub-command>`), no changes are required.

The tests can then be launched through the `yarn test` command, it will find all files with a filename ending in `.test.ts` in the repo and execute them.
- If you are adding a command for a new product, you should:
- Create a label [here](https://github.com/DataDog/datadog-ci/issues/labels) and add it to [`pr-required-labels.yml`](.github/workflows/pr-required-labels.yml).
- Update [`advanced-issue-labeler.yml`](.github/advanced-issue-labeler.yml).
- Update the `changelog` configuration in [`release.yml`](.github/release.yml).

### Continuous Integration tests

Expand Down
2 changes: 1 addition & 1 deletion src/commands/git-metadata/api.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export const datadogSite = process.env.DATADOG_SITE || 'datadoghq.com'
export const datadogSite = process.env.DATADOG_SITE || process.env.DD_SITE || 'datadoghq.com'

export const apiHost = 'api.' + datadogSite

Expand Down
2 changes: 1 addition & 1 deletion src/commands/git-metadata/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class UploadCommand extends Command {
}

const metricsLogger = getMetricsLogger({
datadogSite: process.env.DATADOG_SITE,
datadogSite: process.env.DATADOG_SITE || process.env.DD_SITE,
defaultTags: [`cli_version:${this.cliVersion}`],
prefix: 'datadog.ci.report_commits.',
})
Expand Down
75 changes: 71 additions & 4 deletions src/commands/react-native/__tests__/xcode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ beforeEach(() => {
delete process.env.SOURCEMAP_FILE
delete process.env.UNLOCALIZED_RESOURCES_FOLDER_PATH
delete process.env.USE_HERMES
delete process.env.SKIP_BUNDLING
delete process.env.PLATFORM_NAME
delete process.env.FORCE_BUNDLING
reactNativeVersionSpy.mockClear()
})

Expand Down Expand Up @@ -59,6 +62,7 @@ const basicEnvironment = {
MARKETING_VERSION: '0.0.2',
PRODUCT_BUNDLE_IDENTIFIER: 'com.myapp.test',
SOURCEMAP_FILE: './src/commands/react-native/__tests__/fixtures/basic-ios/main.jsbundle.map',
PLATFORM_NAME: 'iphoneos',
}

const runCLI = async (
Expand Down Expand Up @@ -309,14 +313,15 @@ describe('xcode', () => {
expect(output).toContain('version: 0.0.2 build: 000020 service: com.myapp.test')
})

test('should not compose sourcemaps when using hermes and source maps are not uploaded', async () => {
test('should not bundle RN code when using hermes and building for simulator in debug mode', async () => {
process.env = {
...process.env,
...basicEnvironment,
CONFIGURATION_BUILD_DIR: './src/commands/react-native/__tests__/fixtures/compose-sourcemaps',
UNLOCALIZED_RESOURCES_FOLDER_PATH: 'MyApp.app',
USE_HERMES: 'true',
CONFIGURATION: 'Debug',
PLATFORM_NAME: 'iphonesimulator',
}
const {context, code} = await runCLI(
'./src/commands/react-native/__tests__/fixtures/bundle-script/successful_script.sh',
Expand All @@ -331,15 +336,16 @@ describe('xcode', () => {

expect(code).toBe(0)
const output = context.stdout.toString()
expect(output).toContain('Build configuration Debug is not Release, skipping sourcemaps upload')
expect(output).toContain('Skipping bundling and sourcemaps upload.')
expect(output).not.toContain('Hermes detected, composing sourcemaps')
})

test('should not bundle nor upload sourcemaps when the build configuration is Debug', async () => {
test('should not bundle nor upload sourcemaps when the build configuration is Debug and target is simulator', async () => {
process.env = {
...process.env,
...basicEnvironment,
CONFIGURATION: 'Debug',
PLATFORM_NAME: 'iphonesimulator',
}
const {context, code} = await runCLI(
'./src/commands/react-native/__tests__/fixtures/bundle-script/successful_script.sh'
Expand All @@ -350,7 +356,68 @@ describe('xcode', () => {

expect(code).toBe(0)
const output = context.stdout.toString()
expect(output).toContain('Build configuration Debug is not Release, skipping sourcemaps upload')
expect(output).toContain('Skipping bundling and sourcemaps upload.')
expect(output).not.toContain('Starting successful script')
})

test('should bundle but not upload sourcemaps when the build configuration is Debug and target is phone', async () => {
process.env = {
...process.env,
...basicEnvironment,
CONFIGURATION: 'Debug',
PLATFORM_NAME: 'iphoneos',
}
const {context, code} = await runCLI(
'./src/commands/react-native/__tests__/fixtures/bundle-script/successful_script.sh'
)
// Uncomment these lines for debugging failing script
// console.log(context.stdout.toString())
// console.log(context.stderr.toString())

expect(code).toBe(0)
const output = context.stdout.toString()
expect(output).toContain('Starting successful script')
expect(output).toContain('Build configuration Debug is not Release, skipping sourcemaps upload.')
})

test('should bundle but not upload sourcemaps when the build configuration is Debug and user enforce bundling', async () => {
process.env = {
...process.env,
...basicEnvironment,
CONFIGURATION: 'Debug',
PLATFORM_NAME: 'iphonesimulator',
FORCE_BUNDLING: 'true',
}
const {context, code} = await runCLI(
'./src/commands/react-native/__tests__/fixtures/bundle-script/successful_script.sh'
)
// Uncomment these lines for debugging failing script
// console.log(context.stdout.toString())
// console.log(context.stderr.toString())

expect(code).toBe(0)
const output = context.stdout.toString()
expect(output).toContain('Starting successful script')
expect(output).toContain('Build configuration Debug is not Release, skipping sourcemaps upload.')
})

test('should not bundle nor upload sourcemaps when user skips bundling', async () => {
process.env = {
...process.env,
...basicEnvironment,
CONFIGURATION: 'Release',
SKIP_BUNDLING: 'true',
}
const {context, code} = await runCLI(
'./src/commands/react-native/__tests__/fixtures/bundle-script/successful_script.sh'
)
// Uncomment these lines for debugging failing script
// console.log(context.stdout.toString())
// console.log(context.stderr.toString())

expect(code).toBe(0)
const output = context.stdout.toString()
expect(output).toContain('Skipping bundling and sourcemaps upload.')
expect(output).not.toContain('Starting successful script')
})

Expand Down
56 changes: 47 additions & 9 deletions src/commands/react-native/xcode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,24 @@ export class XCodeCommand extends Command {
return 1
}

if (!this.shouldUploadSourcemaps()) {
this.context.stdout.write(
`Build configuration ${process.env.CONFIGURATION} is not Release, skipping sourcemaps upload`
)

return 0
}

// Run bundle script
try {
if (!this.shouldBundleRNCode()) {
this.context.stdout.write(`Skipping bundling and sourcemaps upload.`)

return 0
}

await this.bundleReactNativeCodeAndImages()

if (!this.shouldUploadSourcemaps()) {
this.context.stdout.write(
`Build configuration ${process.env.CONFIGURATION} is not Release, skipping sourcemaps upload.`
)

return 0
}

/**
* Because of a bug in React Native (https://github.com/facebook/react-native/issues/34212), the composition
* of the 2 Hermes sourcemaps is not done correctly. Therefore we need to do the composition ourselves to
Expand Down Expand Up @@ -437,5 +443,37 @@ export class XCodeCommand extends Command {
return !!podfileLockContent.match('hermes-engine')
}

private shouldUploadSourcemaps = (): boolean => process.env.CONFIGURATION === 'Release' || this.force
private shouldUploadSourcemaps = (): boolean => {
// If we did not bundle the RN code, we won't have anything to upload.
if (!this.shouldBundleRNCode()) {
return false
}

if (this.force) {
return true
}

// We don't upload sourcemaps if the configuration is "Debug"
return !process.env.CONFIGURATION?.includes('Debug')
}

private shouldBundleRNCode = (): boolean => {
if (this.force) {
return true
}

// We keep the same logic and order than react-native-xcode.sh script from RN.
if (!!process.env.SKIP_BUNDLING) {
return false
}
if (process.env.CONFIGURATION?.includes('Debug')) {
// We don't build for simulators in debug mode but we do for real devices.
// See https://github.com/DataDog/expo-datadog/issues/31
if (process.env.PLATFORM_NAME?.includes('simulator')) {
return !!process.env.FORCE_BUNDLING
}
}

return true
}
}
Loading

0 comments on commit de2cabe

Please sign in to comment.