feat: Add basic JS/Jest basic support (WIP)#86
Conversation
cbasitodx
added
enhancement
anmarchenko
left a comment
anmarchenko
left a comment
There was a problem hiding this comment.
I did a first pass, it has the right direction and there are 2 main improvement areas:
- I am not convinced that we really need exclude pattern for frameworks - it makes the implementation more complex than needed
- somehow there are a lot of duplicated code in the framework.go that repeats what we already have and call in
discovery.gopackage
|
|
||
| var ErrFullTestDiscoveryUnsupported = errors.New("full test discovery is not supported") | ||
|
|
||
| type FullTestDiscoverySupporter interface { |
There was a problem hiding this comment.
this smells like an unneeded abstraction layer: we can and should add SupportsFullTestDiscovery to Framework interface and implement it for all frameworks
| // If it implements the FullTestDiscoverySupporter interface, then | ||
| // delegate the choice to SupportFullTestDiscovery() | ||
| func SupportsFullTestDiscovery(f Framework) bool { | ||
| supporter, ok := f.(FullTestDiscoverySupporter) |
There was a problem hiding this comment.
we should avoid typecasting - if we remove FullTestDiscoverySupporter interface, this logic will not be needed
| } | ||
|
|
||
| // cleanupDiscoveryFile removes the discovery file, ignoring "not exists" errors | ||
| func cleanupDiscoveryFile(filePath string) { |
There was a problem hiding this comment.
it belongs to discovery package (and I think it already exists there)
| } | ||
|
|
||
| // executeDiscoveryCommand runs the discovery command and logs timing | ||
| func executeDiscoveryCommand(ctx context.Context, executor ext.CommandExecutor, name string, args []string, envMap map[string]string, frameworkName string) ([]byte, error) { |
There was a problem hiding this comment.
it also exists in discovery package and I don't think it is needed for JS
| } | ||
|
|
||
| // parseDiscoveryFile reads and parses the test discovery JSON file | ||
| func parseDiscoveryFile(filePath string) ([]testoptimization.Test, error) { |
There was a problem hiding this comment.
same as before - reuse discovery package
| return filepath.Join(rootDir, "**", filePattern) | ||
| } | ||
|
|
||
| func globTestFiles(pattern string) ([]string, error) { |
There was a problem hiding this comment.
we already have discovery.DiscoverTestFiles function and we already call it in the planner:
// Goroutine 3: Test files discovery (fast, must always complete)
g.Go(func() error {
startTime := time.Now()
slog.Info("Discovering test files (fast)...", "framework", framework.Name())
var res []string
if resolvedTestFiles.UseExplicitFiles() {
res = resolvedTestFiles.ExplicitFiles
} else {
var discErr error
res, discErr = discovery.DiscoverTestFiles(resolvedTestFiles.Pattern, settings.GetTestsExcludePattern())
if discErr != nil {
fastDiscoveryErr = discErr
slog.Warn("Fast test discovery failed", "error", discErr)
return nil // Don't fail the entire process if full discovery succeeded
}
}
discoveredTestFiles = res
slog.Info("Discovered test files (fast)", "duration", time.Since(startTime), "count", len(discoveredTestFiles))
return nil
})
| // BaseDiscoveryEnv returns environment variables required for all test discovery processes. | ||
| // These env vars ensure the test framework runs in discovery mode without requiring | ||
| // actual Datadog credentials or agent connectivity. | ||
| func BaseDiscoveryEnv() map[string]string { |
There was a problem hiding this comment.
this already lives in discovery.go:
// BaseEnv returns environment variables required for all test discovery processes.
// These env vars ensure the test framework runs in discovery mode without requiring
// actual Datadog credentials or agent connectivity.
func BaseEnv() map[string]string {
return map[string]string{
"DD_CIVISIBILITY_ENABLED": "1",
"DD_CIVISIBILITY_AGENTLESS_ENABLED": "true",
"DD_API_KEY": "dummy_key",
"DD_TEST_OPTIMIZATION_DISCOVERY_ENABLED": "1",
"DD_TEST_OPTIMIZATION_DISCOVERY_FILE": TestsFilePath,
}
}
|
|
||
| var ( | ||
| jestTestFileExtensions = []string{"js", "jsx", "ts", "tsx", "mjs", "cjs"} | ||
| jestExcludedDirs = []string{"node_modules", ".git", "dist", "build", "coverage", ".next"} |
There was a problem hiding this comment.
why do we use excluded dirs for jest by default and not using the "**/{__tests__/**/*,*.spec,*.test}.{ts,js,tsx,jsx}" pattern?
| return nil | ||
| } | ||
|
|
||
| func effectiveTestExcludePattern(testFramework framework.Framework) string { |
There was a problem hiding this comment.
could we avoid this with more precise pattern for jest tests? I think merging exclude patterns could backfire
…est_basic_capabilities_clean
This comment has been minimized.
This comment has been minimized.
2 participants
What does this PR do?
Adds initial JavaScript/Jest support to
ddtest.This includes:
javascriptplatform detection and sanity checks for Node.js anddd-trace/ci/initNODE_OPTIONS=-r dd-trace/ci/initinjection fordd-trace-jsinstrumentationjestframework integration with default Jest test include/exclude patternsnode_modules/.bin/jest, custom command override, ornpx jestMotivation
ddtestcurrently supports Ruby workloads, but Test Optimization also needs JavaScript coverage. This PR starts that support by adding Jest integration, enablingddtestto plan and run JavaScript test files while automatically initializingdd-trace-js.How to test the change?
Run the focused package tests:
go test ./internal/platform ./internal/framework ./internal/planner ./internal/runnerRun the full test suite:
go test ./...