From 52a6d8681eaa33201dc369a4460a37aa848e8cd8 Mon Sep 17 00:00:00 2001 From: Krzysztof Gogolewski <krzysztof.gogolewski@tweag.io> Date: Tue, 18 Jul 2023 12:00:15 +0200 Subject: [PATCH] Testsuite cleanup - Remove misleading help text in perf_notes, ways are not metrics - Remove no_print_summary - this was used for Phabricator - In linters tests, run 'git ls-files' just once. Previously, it was called on each has_ls_files() - Add ghc-prim.cabal to gitignore, noticed in #23726 - Remove ghc-prim.cabal, it was accidentally committed in 524c60c8cd --- .gitignore | 1 + libraries/ghc-prim/ghc-prim.cabal | 108 ------------------------- testsuite/driver/perf_notes.py | 2 +- testsuite/driver/runtests.py | 4 +- testsuite/driver/testglobals.py | 9 --- testsuite/driver/testlib.py | 23 ++++-- testsuite/mk/test.mk | 4 - testsuite/tests/diagnostic-codes/all.T | 11 +-- testsuite/tests/linters/all.T | 30 +++---- 9 files changed, 33 insertions(+), 159 deletions(-) delete mode 100644 libraries/ghc-prim/ghc-prim.cabal diff --git a/.gitignore b/.gitignore index 9b641f920269..bdc915aed863 100644 --- a/.gitignore +++ b/.gitignore @@ -166,6 +166,7 @@ _darcs/ /libraries/ghc-boot-th/ghc-boot-th.cabal /libraries/ghc-boot-th/ghc.mk /libraries/ghc-heap/ghc-heap.cabal +/libraries/ghc-prim/ghc-prim.cabal /libraries/ghci/GNUmakefile /libraries/ghci/ghci.cabal /libraries/ghci/ghc.mk diff --git a/libraries/ghc-prim/ghc-prim.cabal b/libraries/ghc-prim/ghc-prim.cabal deleted file mode 100644 index 345d91265c79..000000000000 --- a/libraries/ghc-prim/ghc-prim.cabal +++ /dev/null @@ -1,108 +0,0 @@ -cabal-version: 2.2 -name: ghc-prim -version: 0.10.0 --- NOTE: Don't forget to update ./changelog.md -license: BSD-3-Clause -license-file: LICENSE -category: GHC -maintainer: libraries@haskell.org -bug-reports: https://gitlab.haskell.org/ghc/ghc/issues/new -synopsis: GHC primitives -build-type: Custom -description: - This package contains the primitive types and operations supplied by GHC. - - It is an internal package, only for the use of GHC developers. - GHC users should not use it! If you do use it then expect - breaking changes at any time without warning. You should prefer - to import @GHC.Exts@ from the @base@ package instead. - -extra-source-files: changelog.md - -source-repository head - type: git - location: https://gitlab.haskell.org/ghc/ghc.git - subdir: libraries/ghc-prim - -custom-setup - setup-depends: base >= 4 && < 5, process, filepath, directory, Cabal >= 1.23 && < 3.9 - -flag need-atomic - default: False - -Library - default-language: Haskell2010 - other-extensions: - BangPatterns - CPP - DeriveGeneric - MagicHash - MultiParamTypeClasses - NoImplicitPrelude - StandaloneDeriving - Trustworthy - TypeFamilies - UnboxedTuples - UnliftedFFITypes - - build-depends: rts == 1.0.* - - exposed-modules: - GHC.CString - GHC.Classes - GHC.Debug - GHC.Magic - GHC.Magic.Dict - GHC.Prim.Ext - GHC.Prim.Panic - GHC.Prim.Exception - GHC.Prim.PtrEq - GHC.PrimopWrappers - GHC.Tuple - GHC.Tuple.Prim - GHC.Types - - virtual-modules: - GHC.Prim - - -- OS Specific - if os(windows) - -- Windows requires some extra libraries for linking because the RTS - -- is no longer re-exporting them (see #11223) - -- ucrt: standard C library. The RTS will automatically include this, - -- but is added for completeness. - -- mingwex: provides GNU POSIX extensions that aren't provided by ucrt. - -- mingw32: Unfortunately required because of a resource leak between - -- mingwex and mingw32. the __math_err symbol is defined in - -- mingw32 which is required by mingwex. - -- user32: provides access to apis to modify user components (UI etc) - -- on Windows. Required because of mingw32. - extra-libraries: user32, mingw32, mingwex, ucrt - - if os(linux) - -- we need libm, but for musl and other's we might need libc, as libm - -- is just an empty shell. - extra-libraries: c, m - - if flag(need-atomic) - -- for 64-bit atomic ops on armel (#20549) - extra-libraries: atomic - - if !os(ghcjs) - c-sources: - cbits/atomic.c - cbits/bswap.c - cbits/bitrev.c - cbits/clz.c - cbits/ctz.c - cbits/debug.c - cbits/longlong.c - cbits/mulIntMayOflo.c - cbits/pdep.c - cbits/pext.c - cbits/popcnt.c - cbits/word2float.c - - -- We need to set the unit ID to ghc-prim (without a version number) - -- as it's magic. - ghc-options: -this-unit-id ghc-prim diff --git a/testsuite/driver/perf_notes.py b/testsuite/driver/perf_notes.py index 3f67f0d57c35..3e4aab6c05af 100644 --- a/testsuite/driver/perf_notes.py +++ b/testsuite/driver/perf_notes.py @@ -645,7 +645,7 @@ def main() -> None: group.add_argument("--metric", help="Test metric (one of " + str(testing_metrics()) + ").") group.add_argument("--way", - help="Test way (one of " + str(testing_metrics()) + ").") + help="Test way (for example, optasm).") group = parser.add_argument_group(title='Plotting', description="Plot historical performance metrics") group.add_argument("--chart", nargs='?', default=None, action='store', const='./PerformanceChart.html', diff --git a/testsuite/driver/runtests.py b/testsuite/driver/runtests.py index e5cec999d6ed..fc0c38360cf5 100644 --- a/testsuite/driver/runtests.py +++ b/testsuite/driver/runtests.py @@ -73,7 +73,6 @@ parser.add_argument("--metrics-file", help="file in which to save (append) the p parser.add_argument("--summary-file", help="file in which to save the (human-readable) summary") parser.add_argument("--unexpected-output-dir", help="directory in which to place unexpected output") parser.add_argument("--target-wrapper", help="wrapper executable to use when executing binaries compiled for the target") -parser.add_argument("--no-print-summary", action="store_true", help="should we print the summary?") parser.add_argument("--only", action="append", help="just this test (can be give multiple --only= flags)") parser.add_argument("--way", action="append", help="just this way") parser.add_argument("--skipway", action="append", help="skip this way") @@ -119,7 +118,6 @@ if args.rootdir: config.metrics_file = args.metrics_file hasMetricsFile = config.metrics_file is not None config.summary_file = args.summary_file -config.no_print_summary = args.no_print_summary config.baseline_commit = args.perf_baseline config.target_wrapper = args.target_wrapper @@ -587,7 +585,7 @@ else: print(Perf.allow_changes_string([(m.change, m.stat) for m in t.metrics])) print('-' * 25) - summary(t, sys.stdout, config.no_print_summary, config.supports_colors) + summary(t, sys.stdout, color=config.supports_colors) # Write perf stats if any exist or if a metrics file is specified. stats_metrics = [stat for (_, stat, __) in t.metrics] # type: List[PerfStat] diff --git a/testsuite/driver/testglobals.py b/testsuite/driver/testglobals.py index c70aab0bc9de..611d531faf58 100644 --- a/testsuite/driver/testglobals.py +++ b/testsuite/driver/testglobals.py @@ -70,15 +70,6 @@ class TestConfig: # Was the compiler compiled with LLVM? self.ghc_built_by_llvm = False - # Should we print the summary? - # Disabling this is useful for Phabricator/Harbormaster - # logfiles, which are truncated to 30 lines. TODO. Revise if - # this is still true. - # Note that we have a separate flag for this, instead of - # overloading --verbose, as you might want to see the summary - # with --verbose=0. - self.no_print_summary = False - # What platform are we running on? self.platform = '' self.os = '' diff --git a/testsuite/driver/testlib.py b/testsuite/driver/testlib.py index 4bd57072c56b..e4bb4154a4fa 100644 --- a/testsuite/driver/testlib.py +++ b/testsuite/driver/testlib.py @@ -364,6 +364,23 @@ def req_host_target_ghc( name, opts ): if isCross() and not js_arch(): opts.skip = True +has_ls_files = None + +# Check that ls-files works and returns files from the source tree. +# We just check that "hie.yaml" is there because it's top-level (don't have to deal with +# path differences) and quite unique to GHC. +def req_ls_files(name, opts): + global has_ls_files + if has_ls_files is None: + try: + files = subprocess.check_output(['git', 'ls-files']).splitlines() + has_ls_files = b"hie.yaml" in files + except subprocess.CalledProcessError: + has_ls_files = False + + if not has_ls_files: + skip(name, opts) + def ignore_stdout(name, opts): opts.ignore_stdout = True @@ -2944,17 +2961,13 @@ def findTFiles(roots: List[str]) -> Iterator[str]: # ----------------------------------------------------------------------------- # Output a test summary to the specified file object -def summary(t: TestRun, file: TextIO, short=False, color=False) -> None: +def summary(t: TestRun, file: TextIO, color=False) -> None: file.write('\n') printUnexpectedTests(file, [t.unexpected_passes, t.unexpected_failures, t.unexpected_stat_failures, t.framework_failures]) - if short: - # Only print the list of unexpected tests above. - return - if len(t.unexpected_failures) > 0 or \ len(t.unexpected_stat_failures) > 0 or \ len(t.unexpected_passes) > 0 or \ diff --git a/testsuite/mk/test.mk b/testsuite/mk/test.mk index 7b346376aa78..505d73173b33 100644 --- a/testsuite/mk/test.mk +++ b/testsuite/mk/test.mk @@ -287,10 +287,6 @@ ifneq "$(SUMMARY_FILE)" "" RUNTEST_OPTS += \ --summary-file "$(SUMMARY_FILE)" endif -ifeq "$(NO_PRINT_SUMMARY)" "YES" -RUNTEST_OPTS += \ - --no-print-summary -endif ifneq "$(PERF_BASELINE_COMMIT)" "" RUNTEST_OPTS += \ diff --git a/testsuite/tests/diagnostic-codes/all.T b/testsuite/tests/diagnostic-codes/all.T index 30fd3c97f775..a3c3f2080dd1 100644 --- a/testsuite/tests/diagnostic-codes/all.T +++ b/testsuite/tests/diagnostic-codes/all.T @@ -1,12 +1,3 @@ - -# Copied from linters/all.T: -def has_ls_files() -> bool: - try: - files = subprocess.check_output(['git', 'ls-files']).splitlines() - return b"hie.yaml" in files - except subprocess.CalledProcessError: - return False - -test('codes', [ normal if has_ls_files() else skip +test('codes', [ req_ls_files , req_hadrian_deps(["lint:codes"]) ] , makefile_test, ['codes']) diff --git a/testsuite/tests/linters/all.T b/testsuite/tests/linters/all.T index 0e06df6d501a..6e655abd09ee 100644 --- a/testsuite/tests/linters/all.T +++ b/testsuite/tests/linters/all.T @@ -1,43 +1,35 @@ -test('uniques', [no_deps, extra_files(["checkUniques"])], makefile_test, ['uniques']) - def normalise_nos(s): return re.sub(r':\d+:\d+','<line>:<no>', s) -# Check that ls-files works and returns files from the source tree. -# We just check that "hie.yaml" is there because it's top-level (dont have have to deal with -# path differences) and quite unique to GHC. -def has_ls_files() -> bool: - try: - files = subprocess.check_output(['git', 'ls-files']).splitlines() - return b"hie.yaml" in files - except subprocess.CalledProcessError: - return False - -test('makefiles', [ no_deps if has_ls_files() else skip +setTestOpts(no_deps) # linters don't need GHC to be built + +test('uniques', [extra_files(["checkUniques"])], makefile_test, ['uniques']) + +test('makefiles', [ req_ls_files , extra_files(["regex-linters"]) ] , makefile_test, ['makefiles']) -test('changelogs', [ no_deps if has_ls_files() else skip +test('changelogs', [ req_ls_files , extra_files(["regex-linters"]) ] , makefile_test, ['changelogs']) -test('cpp', [ no_deps if has_ls_files() else skip +test('cpp', [ req_ls_files , extra_files(["regex-linters"]) ] , makefile_test, ['cpp']) -test('rts-includes', [ no_deps if has_ls_files() else skip +test('rts-includes', [ req_ls_files , extra_files(["regex-linters"]) ] , makefile_test, ['rts-includes']) -test('version-number', [ no_deps if has_ls_files() else skip +test('version-number', [ req_ls_files , extra_files(["regex-linters"]) ] , makefile_test, ['version-number']) -test('notes', [ no_deps if has_ls_files() else skip +test('notes', [ req_ls_files , req_hadrian_deps(["lint:notes"]) , normalise_fun(normalise_nos) ] , makefile_test, ['notes']) -test('whitespace', [ no_deps if has_ls_files() else skip +test('whitespace', [ req_ls_files , req_hadrian_deps(["lint:whitespace"]) ] , makefile_test, ['whitespace']) -- GitLab