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

tests: refactor config tests service name #3777

Merged
merged 5 commits into from
Dec 6, 2023

Conversation

david-luna
Copy link
Member

@david-luna david-luna commented Dec 5, 2023

This PR is refactoring all test related to the resolution of the service name and version. It also moves the test/fixtures/pkg-zero-conf-* folders used to test into test/config/fixtures folder to keep al related files in the same scope.

Checklist

  • Implement code
  • refactor tests
  • Update TypeScript typings
  • Update documentation
  • Add CHANGELOG.asciidoc entry
  • Commit message follows commit guidelines

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Dec 5, 2023
const test = require('tape');

const { normalize } = require('../../lib/config/config');
const { CONFIG_SCHEMA, getDefaultOptions } = require('../../lib/config/schema');

const { runTestFixtures } = require('../_utils');
const { reviver, replacer } = require('./_json-utils.js');
const { reviver, replacer } = require('./json-utils.js');
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for reviewer: nit from the previous PR #3768

@@ -448,7 +452,7 @@ const testFixtures = [
t.deepEqual(
resolvedConfig.globalLabels,
pairs,
'globalLabels is parsed correctly from environment (array)',
'globalLabels is parsed correctly from start options (array)',
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for reviewer: nit from the previous PR #3768

@@ -577,7 +581,7 @@ const testFixtures = [
t.deepEqual(
resolvedConfig.ignoreUrlStr,
['str1'],
'string items of ignoreUrl are added to the right config (ignoreUrlStr)',
'string items of ignoreUrls are added to the right config (ignoreUrlStr)',
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for reviewer: nit from the previous PR #3768

{
name: 'pkg-zero-conf-none - serviceName/serviceVersion resolved to unknown if no package.json present',
script: (function () {
const path = require('path');
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for reviewer: when this iife executes the requires at the top are not resolved yet. So we need to these ones

@david-luna david-luna marked this pull request as ready for review December 5, 2023 18:00
@david-luna david-luna requested a review from trentm December 5, 2023 18:00
@david-luna david-luna merged commit 38e9a8e into main Dec 6, 2023
24 checks passed
@david-luna david-luna deleted the dluna/refactor-config-tests-service-name branch December 6, 2023 18:05
PeterEinberger pushed a commit to fpm-git/apm-agent-nodejs that referenced this pull request Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants