Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39894 )
Change subject: tests: Add build subsystem for unit testing coreboot ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39894/2/tests/Makefile.inc File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39894/2/tests/Makefile.inc@4 PS2, Line 4: ## This program is free software; you can redistribute it and/or modify : ## it under the terms of the GNU General Public License as published by : ## the Free Software Foundation; version 2 of the License. : ## : ## This program is distributed in the hope that it will be useful, : ## but WITHOUT ANY WARRANTY; without even the implied warranty of : ## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : ## GNU General Public License for more details. We're dropping these blocks now, so replace it with # SPDX-License-Identifier: GPL-2.0-only
https://review.coreboot.org/c/coreboot/+/39894/2/tests/Makefile.inc@18 PS2, Line 18: tests $(testobj) since we try to keep the source tree clean.
https://review.coreboot.org/c/coreboot/+/39894/2/tests/Makefile.inc@27 PS2, Line 27: TEST_KCONFIG_FLAGS:= DOTCONFIG=$(TEST_DOTCONFIG) \ it's possible to override variables per-target. This seems to be used only in line 132, could you add these variable overrides to line 130 then? That makes it clear that there's a variable override going on for that target only.
https://review.coreboot.org/c/coreboot/+/39894/2/tests/Makefile.inc@35 PS2, Line 35: tab for consistency
https://review.coreboot.org/c/coreboot/+/39894/2/tests/Makefile.inc@52 PS2, Line 52: # Include Makefile.inc, clean test-specific variables; : # Add directory prefix to all test-specific variables and test name : # itself in order to allow the same test name in different directories : includemakefiles= \ : $(foreach value,tests test-subdirs, \ : $(eval $(value):=)) \ : $(eval -include $(1)) \ : $(foreach test, $(tests), \ : $(foreach attribute, $(attributes), \ : $(eval $(addprefix $(dir $(1)), $(test)-$(attribute)):= \ : $($(test)-$(attribute))))) \ : $(foreach test, $(tests), \ : $(foreach attribute, $(attributes), \ : $(eval $(test)-$(attribute):=))) \ : $(eval alltests+=$(addprefix $(dir $(1)), $(tests))) \ : $(eval subdirs+=$(subst $(CURDIR)/,, \ : $(abspath $(addprefix $(dir $(1)),$(test-subdirs))))) : : # Traverse through the unit-tests tree and invoke includemakefiles : evaluate_subdirs= \ : $(eval cursubdirs:=$(subdirs)) \ : $(eval subdirs:=) \ : $(foreach dir,$(cursubdirs), \ : $(eval $(call includemakefiles,$(dir)/Makefile.inc))) \ : $(if $(subdirs),$(eval $(call evaluate_subdirs))) this is copied from the toplevel Makefile. Isn't this always called in the context of that one, so we don't need the duplication?