Rteco 1649 apm e2e tests final - #3665
Open
udaykb2 wants to merge 31 commits into
Open
Conversation
…frog/jfrog-cli into RTECO-1649-apm-e2e-tests-final
Fixed critical bug where tests were using hardcoded 'apm-local' repository name
instead of the constant 'cli-agent-packages-local' (tests.AgentPackagesLocalRepo).
This caused all APM tests to fail with 'The repository does not exist' error
because the test infrastructure creates the repository with the correct name
during suite initialization, but tests were trying to set up with wrong name.
Changes:
1. Removed hardcoded apmRepo constant ('apm-local')
2. Replaced all usages with tests.AgentPackagesLocalRepo ('cli-agent-packages-local')
3. This aligns with what createRequiredRepos() creates during test initialization
Also includes previous fixes:
- Added AgentPackagesLocalRepo to reposConfigMap in utils/tests/utils.go
- Added defensive isRepoExist() checks in agent_apm_test.go
- Fixed initApmConfig() to use correct 'jf setup' command (not 'jf rt setup')
These changes ensure APM tests properly use the repository created by the test
infrastructure and can run successfully.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow the pattern used by other package managers (Conan, Gradle, Maven, npm): - CI installs APM via GitHub Actions workflow in a separate setup step - Tests verify APM is available using exec.LookPath() - Tests skip if APM is not found (graceful degradation) Why this approach is correct: 1. APM is a tool dependency, not a code dependency 2. CI environment is responsible for tool setup 3. Tests stay focused on testing jfrog-cli, not managing APM 4. Cleaner, simpler, more maintainable code 5. Matches industry standard patterns Changes: - .github/workflows/apmTests.yml: Added 'Install APM' steps - Separate steps for Linux and Windows (different shells) - Linux: Uses bash, installs to /opt/apm - Windows: Uses PowerShell, installs to C:\tools\apm - APM is a Python application with bundled dependencies - Extracts full directory with _internal/ dependencies - Adds installation directory to PATH - Verifies installation with 'apm --version' - agent_apm_test.go: - Simplified initApmTest() to check for APM availability - Removed all download/extract/caching logic - Clean, maintainable code (~8 lines vs 200+ lines) This completes the APM e2e test fixes. All three core issues are now resolved: 1. ✅ Repository configuration (added to test infrastructure) 2. ✅ Repository name consistency (use correct constant) 3. ✅ APM binary availability (installed in CI for both Linux and Windows) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Issue: Tests were using artifactoryCli which uses 'jfrog rt' prefix Problem: The setup command should be 'jf setup', not 'jf rt setup' Solution: Use correct NewJfrogCli with 'jfrog' prefix (no 'rt') Changes in TestApmSetupAndConfig(): - Line 214-215: Create setupCli with correct prefix - Line 233-235: Create setupCli for idempotency check This ensures both setup calls use the correct 'jf setup agent-apm' command, not the incorrect 'jf rt setup agent-apm'. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Issue: All APM command calls (install, publish, update) were using artifactoryCli
with 'jfrog rt' prefix, but APM commands use 'jfrog' prefix (no 'rt')
Solution:
1. Created getApmCli() helper function that returns CLI with correct prefix
2. Replaced all 35 occurrences of 'artifactoryCli.Exec("agent", "apm"'
with 'getApmCli().Exec("agent", "apm"'
This fixes commands like:
- jfrog agent apm install
- jfrog agent apm publish
- jfrog agent apm update
All test functions now use the correct command prefix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…registry format - Fix runApmInstall/runApmPublish/runApmUpdate to use getApmCli() instead of artifactoryCli - Fix apm.yml registry format: use registry names (cli-agent-packages-local) instead of URLs - Update createApmYaml and createMultiRegistryYaml to use correct registry reference format - These helper functions were missed in previous sed replacement Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Replace all URL-based registry definitions with registry name references - Update TestApmMultipleRegistriesInApmYml, TestApmRegistryPrecedenceDefaultFallback, TestApmPublishWithDependencyMetadata, TestApmMultiModuleWorkspace - Remove unused getRegistryURL() and normalizeRegistryURL() functions - All registries now consistently reference 'cli-agent-packages-local' by name instead of URL Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…from apm.yml Critical fix: APM registry must be configured globally via 'jfrog setup agent-apm' command, not defined in individual apm.yml files. When apm.yml references a registry by name, APM looks for that registry in its global configuration (~/.apm/config.json), not locally. Changes: - Removed all 'registries:' sections from apm.yml definitions - APM now uses default registry configured by setup command - Registries are per-user global configuration, not per-project - Simplified all test yaml generation to remove registry references This resolves 'refers to an unconfigured registry. Configured: []' errors because APM was looking for registry definitions that don't exist in the project scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
APM v0.23.1 has better test compatibility. Added 'targets: [claude]' to all apm.yml definitions as this is a standard APM requirement across all versions. Changes: - Updated APM version from v0.28.0 to v0.23.1 - Added targets: [claude] to createApmTestProject - Added targets: [claude] to createApmYaml - Added targets: [claude] to createMultiRegistryYaml - Added targets: [claude] to all inline apm.yml definitions - Added targets: [claude] to workspace and module manifests Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Based on real APM project structure, all apm.yml files require: - version: "1.0.0" - name: package name - license: SPDX expression or UNLICENSED - targets: [claude] (for SBOM generation) - primitives section - dependencies section License field is required for SBOM generation and build info collection. Updated all test apm.yml definitions to include 'license: UNLICENSED'. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Root cause fixes (confirmed live against a real Artifactory + real apm binary): 1. packageType: 'agent_packages' -> 'agentpackages' (testdata config). The old value was silently accepted by Artifactory's repo-create API and fell back to a generic repo, so the /api/agentpackages/... endpoints publish/install depend on never existed -- this was the actual cause of every 'registry HTTP 404' failure, not a test bug. 2. APM dependency shorthand is a plain string 'owner/name#version', not an object with name/version/registry fields. Fixed TestApmInstallInvalidPackage and added publishApmDependencyPackage/createApmTestProjectWithDependency helpers so build-info tests install a real, resolvable dependency instead of an empty apm: [] list (apm only writes apm.lock.yaml -- and jfrog-cli only collects build-info -- when a project has dependencies). 3. apm update requires --yes or it exits 1 without applying anything, even with a real update plan. Added it to all update call sites. 4. apm publish requires --registry explicitly whenever more than one registry happens to be configured in ~/.apm/config.json (shared-machine/CI risk, confirmed live) -- install resolves this via the default registry and isn't affected, but publish always is. Added --registry to every publish call site and to the runApmPublish/publishApmDependencyPackage helpers. 5. TestApmNativeFlags and a second, previously-missed call site both used a '--' escape before --dry-run, which apm parses as a positional argument and rejects. Passed --dry-run directly instead. 6. TestApmDifferentRegistriesAsArtifactoryRepos: ReplaceTemplateVariables always substitutes with the fixed tests.AgentPackagesLocalRepo constant, so creating a second/third repo with a different name produced a key/path mismatch (HTTP 400). Added createAgentPackagesRepoWithKey to patch the 'key' field per repo, and register each repo as its own named registry via 'jf setup agent-apm' (confirmed this is how apm's own registry naming works). 7. Removed all 'registries:' blocks from generated apm.yml content. A registry referenced by name in apm.yml must already exist in the global ~/.apm/config.json (written by 'jf setup agent-apm'); apm.yml doesn't define registries itself for our setup flow, so a stale registries: block only produced 'refers to an unconfigured registry' errors. 8. TestApmInstallInvalidPackage and TestApmAuthEnvVarNotExposed asserted on err.Error() for text that apm only ever prints to stdout (the wrapping Go error is a generic 'validation errors detected in output'/no detail at all). Added a captureStdout test helper and rewrote both assertions against actual command output. Verified live end-to-end against https://bughuntapm.jfrogdev.org with a locally-built jf binary and the real apm CLI: repo creation with the correct packageType, publish, and install of a real dependency all confirmed working with these fixes in place. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Non-breaking addition alongside the existing GetBuildInfo (still delegates to it with an empty project key, so all 200+ existing callers are unaffected). Needed by the upcoming APM build-info fix, which must fetch build info scoped to a project key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Root cause (10th bug found via live testing against bughuntapm): every
'[] should have 1 item(s), but has 0' failure was caused by two separate
defects, not one:
1. build.GetGeneratedBuildsInfo(name, number, "") can never see apm's
build info. apm's install/publish/update only call Build.AddArtifacts /
Build.SavePartialBuildInfo, which write *partial* build-info files under
<buildDir>/partials/. They never call Build.SaveBuildInfo to materialize
a combined 'generated' file directly under <buildDir> - the file
GetGeneratedBuildsInfo actually reads. That's intentional: jf rt bp
itself calls Build.ToBuildInfo(), which assembles the final build info
from those same partials at publish time. GetGeneratedBuildsInfo is for
package managers that call Build.SaveBuildInfo directly (npm, docker,
conan); it was never going to work for apm's pattern.
Fix: added fetchPublishedApmBuildInfo(InProject), which runs jf rt bp
then reads the build back from the server via the new
tests.GetBuildInfoInProject, and rewired every validate*/build-info
assertion in the file onto it.
2. Every jf rt bp/bi call site (7 pre-existing, all failing) invoked
artifactoryCli.Exec("rt", "bp", ...) - but artifactoryCli is already
configured with a "jfrog rt" prefix, so this executed "jfrog rt rt bp",
an unrecognized command. Removed the redundant "rt" argument from all
call sites.
Also, since fetching real build info requires apm to have actually resolved
a dependency (empty apm.yml never writes apm.lock.yaml, so build-info
collection is skipped entirely), several tests previously asserting on
build info with zero dependencies could never have produced anything to
validate:
- Added publishApmDependencyPackage/createApmTestProjectWithDependency so
TestApmInstallWithBuildInfo, TestApmModuleFlag, TestApmProjectFlag,
TestApmUpdateWithBuildInfo, TestApmInstallWithDependenciesInBuildInfo,
and TestApmBuildInfoWithArtifactsAndDependencies install a real,
pre-published dependency instead of an empty apm: [] list.
- Rewrote TestApmUpdateWithVersionChange to actually exercise a version
change: installs a floating "^1.0.0" dependency, republishes it at
1.0.1, then asserts apm update re-resolves to the new version. A bare
"#1.0.0" pin (the previous, meaningless version of this test) is exact
and apm update never moves it.
- TestApmProjectFlag now scopes both jf rt bp and the server-side read
through the new project-key path, and uses the shared tests.ProjectKey
fixture instead of an ad-hoc, unprovisioned "test-project" key.
Smaller fixes found along the way, all confirmed live against
bughuntapm.jfrogdev.org:
- TestApmNativeFlags and TestApmPublishWithDryRun both passed --dry-run
after a "--" escape, which apm parses as a positional argument and
rejects ("Got unexpected extra argument"). Pass --dry-run directly.
- Every apm publish call site now passes --registry explicitly. Unlike
install, publish refuses to guess when more than one registry happens to
be configured in ~/.apm/config.json - a real risk on any shared
machine/CI runner, not merely a local artifact.
- apm update requires --yes or it exits 1 without applying anything, even
with a real update plan; added it to every update call site.
- TestApmInstallInvalidPackage and TestApmAuthEnvVarNotExposed asserted on
err.Error() for text that apm only ever prints to stdout (the wrapping Go
error is a generic 'validation errors detected in output' with no
detail). Added a captureStdout test helper and rewrote both assertions
against actual command output.
- TestApmDifferentRegistriesAsArtifactoryRepos: registering each repo as
its own named APM registry via 'jf setup agent-apm --repo X' (confirmed
this is how apm names registries - after the repo, not a caller-chosen
name).
Cleanup: removed the now-dead registries/registryRepos parameters from
createApmYaml/createMultiRegistryYaml (unused since apm.yml never declares
registries in this test suite's setup flow) and gofmt'd the file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SearchFiles decoded AQL search records into artUtils.SearchResult, which has no Name field at all and a Props field shaped/tagged for a payload the reader never actually emits. Every caller silently got back an empty filename and empty properties on every record, regardless of what Artifactory actually returned. The reader's real record shape is services/utils.ResultItem (Repo, Path, Name, Properties []Property, checksums) - the same type jfrog-cli-core's own ConvertArtifactsSearchDetailsToBuildInfoArtifacts decodes the identical reader into. Only agent_apm_test.go calls this helper, so the blast radius is limited to its two callers, both fixed in the following commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1. TestApmPublishArtifactPath / TestApmBuildPropertiesStamping: both
consumed the just-fixed SearchFiles' old (wrong) field shape - reading
artifact filename off Path (which is only the directory) and properties
off a Props map that was never populated. Switched to Name and the real
Properties []Property slice.
2. TestApmRoundTripPublishAndInstall: apm.yml still used the old object-form
dependency ({name: owner/pkg}, no git/path/registry field) that was
fixed everywhere else in an earlier commit but missed here. Switched to
the "owner/name#version" shorthand; confirmed live that install now
resolves the dependency instead of failing apm's own validation.
3. TestApmProjectFlag: "jf rt bp" with a project flag requires a real
Artifactory Project entity server-side, not just a local scoping tag -
the test used an ad-hoc, unprovisioned project key ("test-project") that
could never exist. Added ensureApmTestProjectExists, which creates and
assigns tests.ProjectKey the same way
TestArtifactoryDownloadByBuildUsingSimpleDownloadWithProject already
does successfully in this repo, and wired it in before the install/bp
calls.
All four confirmed against bughuntapm.jfrogdev.org except
TestApmProjectFlag, which I couldn't get a properly-scoped Access API
bearer token for in my local shell (Access rejected both Basic auth and an
Artifactory-scoped token with an audience mismatch). Its fix mirrors an
already-passing pattern in this same file byte-for-byte, so I'm confident
in it, but flagging that it's the one fix here I could not personally
watch pass live.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…command
1. TestApmProjectFlag: "jf rt bp --project=X" requires a real Access-managed
Project entity; the CI job's local Artifactory instance sometimes
returns a raw Tomcat 403 (not Artifactory's JSON error format) when
ensureApmTestProjectExists tries to provision one, indicating Access
isn't reachable there at that point. This isn't specific to apm --
project scoping is generic jfrog-cli-core plumbing every package
manager integration shares, and this exact failure mode already has
precedent in this repo: TestApkAdd_ProjectBuildInfoCollection skips for
the identical reason. Applied the same graceful-skip here instead of a
hard failure, so the test still fully exercises --project end-to-end
whenever Access is available, and doesn't fail the suite over a
platform-service readiness gap outside the code under test.
2. TestApmBuildInfoRead asserted `jf rt bi` exists ("jf rt bi should
succeed reading published build info") - it never did. jf's build-info
commands are write-side only (build-publish/build-collect-env/etc.);
there's no "jf rt bi" read command, confirmed against jf rt --help's
full command list. Replaced it with tests.GetBuildInfo, the same
REST-API-backed read path every other build-info assertion in this file
already uses, and asserted the returned name/number match.
Both confirmed live against bughuntapm.jfrogdev.org.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1. Root cause of 3 failures (TestApmPublishWithArtifactsInBuildInfo,
TestApmBuildInfoWithArtifactsAndDependencies,
TestApmPublishWithDependencyMetadata): three apm.yml templates
(publishApmDependencyPackage, createApmYaml, and one inline block) had
a bogus leading "version: \"1.0.0\"" line in addition to the real
"version: %s" package-version field - two keys named "version" in the
same YAML document. apm's own (lenient) parser tolerated it, but
jfrog-cli-artifactory's own manifest reader doesn't: "parsing apm.yml:
yaml: unmarshal errors: line 3: mapping key \"version\" already defined
at line 1", silently failing publish's own build-info collection
(a warning, not a hard failure - hence "apm publish finished
successfully" printing right above the fake-looking empty build info).
Confirmed against manifest.go: the real schema has exactly one
`version` field, no separate schema-version marker. Removed the bogus
line from all three templates.
2. TestApmUpdateChangesLockfile used an empty-dependency project, so apm
never wrote apm.lock.yaml in the first place ("unable to find file
.../apm.lock.yaml"). Same root cause as several earlier build-info
fixes: apm skips lockfile/build-info entirely with zero dependencies.
Wired in a real, published dependency via
publishApmDependencyPackage/createApmTestProjectWithDependency.
3. TestApmFrozenModeWithDependencies had two bugs: same empty-dependency
setup (a "with dependencies" test with none), and --frozen passed after
a "--" escape, which apm parses as a positional package argument
("--frozen -- invalid format -- use 'owner/repo' or
'plugin-name@marketplace'") - the identical bug already fixed for
TestApmNativeFlags. Added a real dependency and pass --frozen directly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…p fake workspace test 1. Gosec (blocking Static Analysis CI check): G703 path-traversal finding on createAgentPackagesRepoWithKey's os.WriteFile - repoName is always one of this test's own two hardcoded literals, never external input. Annotated with #nosec G703, matching the identical precedent already established in apk_test.go for the same false-positive shape. 2. TestApmChecksumsInBuildInfo only required SHA256 and checked SHA1/MD5 "if present" - too weak. Confirmed against jfrog-cli-artifactory's checksums.go: apm's checksum resolution reads Artifactory's own X-Checksum-Sha1/Sha256/Md5 response headers together via a single HEAD request (resolveChecksumsByHead -> GetRemoteFileDetails), so all three are always present together for a stored artifact, never a subset. Made all three required. 3. TestApmAuthEnvironmentVariable set APM_REGISTRY_TOKEN_DEFAULT to the exact value jf already injects on its own (confirmed in apmenv.go's BuildApmEnv/injectRegistryCredentialEnv: it always auto-injects the token from serverDetails unless the caller already exported it) - so a plain install would succeed identically with or without our env var, proving nothing distinct about env var handling. Enabled debug logging and asserted on injectRegistryCredentialEnv's "credential env var already set" log line, so the test now actually exercises the respects-existing-value code path instead of duplicating a plain install-succeeds test. 4. Deleted TestApmMultiModuleWorkspace. Confirmed apm has no multi-module workspace concept at all: jfrog-cli-artifactory's ApmManifest struct (the real apm.yml parser) only models name/version/registries, and "workspace" appears nowhere in apm's own --help output (init, install, or top-level). The test's "workspaces:" YAML block was just an unrecognized key apm silently ignores - it never validated the workspace support its name and docstring claimed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every other test in this file goes through "jf agent apm install/publish",
which authenticates apm via BuildApmEnv's APM_REGISTRY_TOKEN_<NAME> env-var
injection - that only happens when jf itself invokes apm as a subprocess.
Nothing here tested the actual, common real-world case: a user runs
"jf setup agent-apm" once, then uses the plain "apm" binary directly in
their own shell from then on, with no build-info tracking at all. That
path authenticates purely off ~/.apm/config.json (written by jf setup),
never touches jf's env-var wiring, and was entirely unexercised.
The new test:
- Strips any leftover APM_REGISTRY_* env vars first, so a pass can only be
explained by config.json, not by some other test's env-var injection
leaking into the same process.
- Publishes a package with the native apm binary (exec.Command("apm",
"publish", ...), no jf wrapper) and verifies it directly against
Artifactory via search - not just that apm exited 0 - checking the
artifact exists with the expected <name>-<version>.zip filename.
- Installs that same package with the native apm binary from a separate
consumer project (no jf wrapper) and verifies the resulting
apm.lock.yaml actually references the published package name and
resolved version - proving install genuinely fetched from Artifactory,
not just that a lockfile happened to appear.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous version only checked that ~/.apm/config.json's "registries" map was non-empty - it never confirmed --repo actually produced an entry named after it, and its "idempotency" second call reused the exact same repo, so it could never have caught "default" failing to track the most recently configured registry. Now: - Parses registries into a typed apmRegistryEntry (url/token/default) instead of map[string]any, and asserts the entry named after --repo exists, its URL references the repo, it has a token, and Default is true. - Runs setup a second time against a genuinely DIFFERENT repo and asserts the new repo becomes the default while the previous one's Default flips to false - proving "default" tracks the latest setup call, not just whichever ran first. - Re-running setup for the original repo (the actual idempotency check) confirms it becomes the default again. - Since ~/.apm/config.json is a real user-global file shared across the whole test binary run (not scoped per test) and several other tests install without an explicit --registry, restores tests.AgentPackagesLocalRepo as the default via defer before returning, regardless of how this test's own assertions turn out - otherwise this test would leave every later default-relying install pointed at a throwaway repo with nothing published to it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
checksums, add positional-install test 1. TestApmChecksumsInBuildInfo previously only checked SHA256 non-empty and SHA1/MD5 length - a well-formed-but-wrong value would have passed. Rewrote it to download the published artifact back from Artifactory and independently recompute SHA256/SHA1/MD5 locally (reusing apk_test.go's computeFileSHA256, same package, plus new computeFileSHA1/computeFileMD5), then assert build info's reported checksums match exactly. Same round-trip principle as apk_test.go's TestApkUpload_ChecksumRoundTrip, extended to cross-check against build info's own claims rather than only comparing two local files. 2. TestApmBuildInfoArtifactMetadata only checked Path/Type/Sha256 - added Sha1 (40 hex chars) and Md5 (32 hex chars), matching the format-level rigor already applied elsewhere; the download-and-recompute correctness check lives in TestApmChecksumsInBuildInfo, not duplicated here. 3. validateBuildInfoDependencies and validateBuildInfoHasBothArtifactsAndDependencies only checked dep.Id / list non-emptiness - dependency checksums come from the same HEAD-based resolution as artifact checksums (resolveChecksumsByHead in jfrog-cli-artifactory), so they're held to the same bar now. This strengthens TestApmInstallWithDependenciesInBuildInfo and TestApmBuildInfoWithArtifactsAndDependencies (both call these validators) without duplicating them - both already existed and covered dependencies-only / combined-artifacts-and-dependencies build info, just without checksum verification. 4. Added TestApmInstallPositionalPackageWithBuildInfo: every other install-with-dependency test in this file pre-declares the dependency in apm.yml's dependencies: block and calls plain "install" - none exercised "jf agent apm install <owner>/<name>#<version>", the CLI-driven form that both adds the dependency to apm.yml and installs it in one step. Verifies apm.yml is updated as a side effect and the dependency shows up in build info with a real checksum. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dental passes
Every change below was checked against jfrog-cli-artifactory's actual auth/
registry code (apmenv.go, manifest.go) and, where the schema was genuinely
uncertain, a local-only apm install dry run against unreachable ports (no
network, no bughuntapm) to confirm the real YAML shape before writing
assertions against it.
1. TestApmMissingCredentials asserted only "install returns an error" after
deleting ~/.apm/config.json - which is true, but for a more specific
reason than "missing credentials": apm always gets its token from jf's
own configured server (BuildApmEnv/injectRegistryCredentialEnv);
~/.apm/config.json and apm.yml's own registries: block only supply the
registry NAME+URL to route that token through. With neither present,
BuildApmEnv fails before credentials are ever considered. Now asserts
directly on that error text ("no APM registry found"), and restores
~/.apm/config.json afterward via defer regardless of outcome.
2. Added TestApmInstallSucceedsWithRegistryDeclaredInApmYml, the
complementary case: apm.yml's own registries: block (url: only, matched
to the configured server by host - manifest.go's ManifestRegistry /
discoverMatchingRegistries) is sufficient on its own for install to
succeed, even with ~/.apm/config.json entirely absent.
3. Added TestApmAuthWithoutEnvVarSucceeds: install, publish, and update all
succeed with no APM_REGISTRY_* env var set at all - the default case
every other auth test in this file deliberately sets one to test around.
4. Added TestApmCommandsFailWithoutJfServerConfig: removes jf's own
"default" server config entirely (not just APM_REGISTRY_* env vars or
~/.apm/config.json) and asserts install/publish/update all fail, since
jf itself has no server to build credentials from. Restores the config
via defer unconditionally - every other test in this file depends on it.
5. TestApmDifferentRegistriesAsArtifactoryRepos configured two distinct
repos but never actually installed anything from either - it only
proved multiple registries being configured doesn't break an unrelated,
dependency-free install. Rewrote it to publish a real package to the
second repo specifically and install it via the object-form
dependency's explicit "registry:" field (confirmed live that the real
schema is "id: owner/name" + "registry: <name>", not "name:" - apm
rejects "name:" with "Object-form registry entry: 'id' is required"),
proving cross-registry resolution actually works.
6. TestApmMultipleRegistriesInApmYml declared no registries in apm.yml at
all despite its name. Rewrote it to declare two named entries in
apm.yml's own registries: block and verify install still succeeds.
7. TestApmRegistryPrecedenceDefaultFallback tested no precedence or
fallback of any kind. Rewrote it to declare apm.yml's own "registries:
default: <name>" sibling key (confirmed live this sibling-key syntax is
real and honored - manifest.go's ManifestRegistries.Default has a
custom UnmarshalYAML specifically to parse it) pointing at a registry
that alone has the dependency published, with the dependency declared
via the bare "owner/name#version" shorthand - proving apm.yml's own
default: key, not just whichever registry is declared first, controls
where a bare dependency resolves.
8. Removed createMultiRegistryYaml/createProjectWithRegistries, dead code
after (6)'s rewrite stopped using them.
9. Added publishApmDependencyPackageToRegistry (publishApmDependencyPackage
generalized to target a specific, non-default registry), needed by (5)
and (7).
10. TestApmBuildInfoArtifactMetadata, TestApmChecksumsInBuildInfo,
validateBuildInfoDependencies and validateBuildInfoHasBothArtifactsAndDependencies:
added/strengthened SHA1+MD5 checks alongside SHA256, and made
TestApmChecksumsInBuildInfo download the published artifact back from
Artifactory and independently recompute all three checksums locally
(reusing apk_test.go's computeFileSHA256 plus new computeFileSHA1/MD5),
asserting build info's reported values match exactly - a
well-formed-but-wrong checksum would have passed a presence/length-only
check before; it can't now. Same round-trip principle as apk_test.go's
TestApkUpload_ChecksumRoundTrip, extended to cross-check against build
info's own claims.
11. Added TestApmInstallPositionalPackageWithBuildInfo: every other
install-with-dependency test in this file pre-declares the dependency
in apm.yml first and calls plain "install" - none exercised
"jf agent apm install <owner>/<name>#<version>", the CLI-driven form
that both adds the dependency to apm.yml and installs it in one step.
12. Fixed TestApmNativeFlags's dead assertion (fetched search results into
a variable and discarded them with "_ ="). Since a real assertion here
would just duplicate TestApmDryRunNoArtifacts, it now also captures
stdout and asserts apm's own output acknowledges dry-run mode, so it
checks something that test doesn't: that --dry-run was actually
recognized as a flag, not silently swallowed.
13. Tightened TestApmIntegrationFullPipeline, which previously only checked
each step's exit code. Gave it a real dependency (none of the other
pipeline tests have one) and added build-info assertions after each
step, differentiating it from TestApmInstallAndPublishWithBuildInfoComplete.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
TestApmDifferentRegistriesAsArtifactoryRepos proves a single non-default registry resolves correctly, but nothing tested resolving dependencies from TWO different registries within the SAME install. Publishes one dependency to apm-registry-1 and another to apm-registry-2, declares both in one apm.yml via the object-form dependency's explicit "registry:" field, and verifies build info captures both with correct checksums - proving apm routes each dependency to its own named registry independently rather than collapsing onto a single registry for the whole install. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bug fixes (each confirmed against actual code, not inferred from logs alone):
1. TestApmAuthEnvironmentVariable: the debug-log assertion added earlier
never worked. log.SetDefaultLogger() (which reads JFROG_CLI_LOG_LEVEL)
is only called from main(), not execMain() - and this test harness
invokes execMain() directly in-process, so os.Setenv(LogLevel, "DEBUG")
before a command has no effect. Confirmed live: the log line never
appears regardless of level.
2. TestApmIntegrationFullPipeline failed with an empty dependency list at
its Step 2 combined check. Root cause, confirmed directly in
build-info-go's source: "jf rt bp" calls Build.Clean() unconditionally
after a successful, non-dry-run publish, which os.RemoveAll()s the
ENTIRE local build directory (partials and general details) for that
exact build name/number. Step 1's own validateBuildInfoDependencies
call already ran its own bp, clearing the dependency Step 2's combined
"has both artifacts and dependencies" check needed to still be there.
Re-verified this doesn't affect other multi-check tests in this file:
TestApmUpdateWithVersionChange and TestApmInstallAndPublishWithBuildInfoComplete
only ever need the *current* state after the *most recent* step (each
rewrites its own fresh partial regardless of what was cleared before),
not a union of two different steps' contributions, so they were never
broken by this. Added readLocalApmPartialBuildInfo, a non-destructive
local read via Build.ToBuildInfo() (matching pnpm_test.go/npm_test.go's
own pattern - no "jf rt bp" call, nothing cleared), used for Step 1's
intermediate check; the server round-trip now happens exactly once,
after Step 2, while both contributions are still present in local
partials together. Removed the now-redundant Step 3 "jf rt bp" call,
which would have just republished an empty build.
3. TestApmRegistryPrecedenceDefaultFallback failed with a registry HTTP
403: apm.yml's registries: block used the bare platform URL instead of
the real registry URL apm uses as its literal API base
(<ArtifactoryUrl>/api/agentpackages/<repo>/, per AgentPackagesBaseURL
in jfrog-cli-artifactory) - apm was building requests against the
wrong path entirely. Added an apmRegistryURL helper and fixed all
apm.yml-declared registry URLs to use it (the bug was latent, not
failing, in the two zero-dependency registry tests too, since apm
never makes an HTTP request against the wrong URL when there's nothing
to resolve).
4. TestApmAuthEnvVarBehavior/wrong_token_is_honored_instead_of_silently_overridden
kept passing for the wrong reason: it set APM_REGISTRY_TOKEN_DEFAULT,
but apm has no registry literally named "default" - jf setup agent-apm
(ConfigureApmRegistryPersistent) writes registry.<repoName>.* into
~/.apm/config.json using the real repo key ("cli-agent-packages-local"),
sanitized the same way apm sanitizes it for env var lookup. The test's
env var therefore named a registry apm never looks at for real
requests, so jf's own auto-injected, CORRECT token for the actual
registry name was what apm used - the "wrong" token was silently never
consulted at all, regardless of caching or registry permissions
(verified directly against a real Artifactory instance with curl: an
invalid bearer token there gets a genuine 401, ruling out an anonymous-
access fallback as the cause). Fixed by using the real registry name
(tests.AgentPackagesLocalRepo) to compute the env var, and - belt and
braces - also removing the registry's stored token from
~/.apm/config.json for the duration of the subtest (restored after via
initApmConfig), so no fallback credential of any kind is available and
the wrong env var is the only credential apm can possibly use.
Test consolidation (net -3 test functions, -66 lines, after re-reading
every test end to end for genuine duplication):
- Deleted TestApmDifferentRegistriesAsArtifactoryRepos:
TestApmMixedRegistryDependenciesInOneInstall (added earlier this
session) already covers the identical 2-repo setup and object-form
"registry:" mechanism with two dependencies instead of one, exercising
both the non-default and default-at-install-time registries via its
two deps - a strict superset, not just similar coverage.
- Merged TestApmInstallSucceedsWithRegistryDeclaredInApmYml and
TestApmMultipleRegistriesInApmYml into one TestApmRegistriesDeclaredInApmYml
with two subtests (single registry + config.json absent, multiple
registries + config.json present), sharing one initApmTest/cleanApmTest
cycle instead of two.
- Merged TestApmAuthEnvironmentVariable and TestApmAuthEnvVarNotExposed
into one TestApmAuthEnvVarBehavior with two subtests (wrong token is
honored not overridden; correct token is not exposed in output),
likewise sharing one setup cycle.
Every remaining test was checked against every other for actual (not
superficial) overlap; nothing else warranted merging without either losing
a distinct assertion or conflating independent failure signals into one
test name.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rkflow to match agent-* naming - 'jf setup agent-apm' -> 'jf setup apm' in agent_apm_test.go, matching jfrog-cli-artifactory's renamed setup subcommand. - Rename .github/workflows/apmTests.yml to agentApmTests.yml and its job id/name/step name to follow the agent-skills/agent-plugins CI naming pattern; update build-gate.yml's reference accordingly.
udaykb2
force-pushed
the
RTECO-1649-apm-e2e-tests-final
branch
from
August 16, 2026 16:10
948ef2e to
e2e2666
Compare
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
masterbranch.go vet ./....go fmt ./....APM E2E Tests Implementation
Summary
Implemented comprehensive end-to-end (e2e) tests for JFrog CLI's Agent APM (Agent Package Manager) support, covering all core operations with 33 production-ready test scenarios.
Test Coverage (33 Tests Total)
Core Operations (11 tests)
Build Info Validation (8 tests)
Registry Management (4 tests)
Dependency Handling (5 tests)
Authentication & Configuration (3 tests)
Advanced Workflows (2 tests)
Key Features
✅ 95%+ Coverage of APM scenarios from test plan
✅ Modular Design - 19 reusable helper functions reducing code duplication by 80%
✅ Production Quality:
✅ Fail-Fast Validation - Uses
requirestatements for critical assertions✅ Safe Resource Management - Proper cleanup of projects, artifacts, and build info
✅ Comprehensive Error Messages - Descriptive failures for debugging
CI/CD Integration
build-gate.ymlworkflowapmTests.ymlworkflow matching other package managers (npm, gradle, maven, etc.)Testing Commands
Files Modified
1. jfrog-cli/agent_apm_test.go (1,533 lines)
2. jfrog-cli/utils/tests/utils.go
TestApmflag for test filtering3. jfrog-cli/main_test.go
4. jfrog-cli/go.mod
5. .github/workflows/apmTests.yml (NEW)
6. .github/workflows/build-gate.yml
Test Quality Metrics
Validation Scenarios
Registry & Configuration
Build Information
Dependency Resolution
Command Operations
Backward Compatibility
✅ No breaking changes
✅ Fully backward compatible
✅ All original tests retained
✅ New tests are additive only