klesh opened a new pull request, #9277:
URL: https://github.com/apache/devlake/pull/9277
### Summary
Nothing enforced the dependency direction between the framework and the
plugins, and two test files had already crossed it:
- `helpers/e2ehelper/data_flow_tester_test.go` imported
`plugins/gitlab/{models,tasks}`
- `server/services/blueprint_makeplan_v200_test.go` imported
`plugins/org/tasks`
This adds `backend/core/layering_test.go`, which reads the import graph of
`core/...`, `helpers/...` and `server/...` — including `_test.go` files — with
`go list` and fails on any `plugins/` edge.
The only tolerated edge is the composition root: `server` may import the
generated `plugins/all` registry, never an individual plugin. That is one
explicit map entry, so any new exception has to be argued for in review.
The rule becomes load-bearing once plugins are compiled into the server
binary by default (#9276): a single framework→plugin import links that plugin
and its whole dependency tree into the core payload and defeats subset builds.
The two offending tests are made plugin-agnostic rather than allowlisted:
- the `e2ehelper` example now uses local fixtures instead of gitlab's
- the blueprint test builds its mocked org task option locally (the plugin
is mocked, so the value only has to round-trip)
A third, long-failing assertion in the same package is corrected in the
second commit.
### Does this close any open issues?
Does not close #9276; it is the first prerequisite of it.
Refs #9276.
### Screenshots
N/A
### Other Information
Two deliberate choices worth a reviewer's attention:
1. **A Go test rather than a `depguard` rule.** `.golangci.yaml` sets
`run.tests: false`, so lint would not have seen either violation that actually
existed — both were in `_test.go` files.
2. **The test opens every framework source file** at the end so the Go test
cache invalidates when an import is added elsewhere. Without that, a plain `go
test ./core` would replay a cached PASS: I verified that adding a violation
under `helpers/` still reported `(cached) ok` for the `core` package before
this was added.
The guard runs as part of `make unit-test`, because
`scripts/unit-test-go.sh` already includes the `core` package.
The second commit fixes `TestGetTableMetaData/dal_get_pk_column_names`:
`GetPrimarykeyColumnNames` has returned qualified `table.column` names since
8e915cd59, and callers strip the prefix themselves
(`helpers/e2ehelper/data_flow_tester.go:316`,
`helpers/srvhelper/scope_config_service_helper.go:74`). The stale assertion
went unnoticed because `scripts/unit-test-go.sh` filters out every package path
matching `e2e`, which covers `helpers/e2ehelper`; that package needs a live
`E2E_DB_URL` and therefore stays out of the DB-less unit-test job.
Verification:
```
go test ./core/ ./helpers/e2ehelper/ ./server/services/ # ok
go test ./core/... ./helpers/... ./server/... # exit 0, no
failures
gofmt -l # clean
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
