Standardize run tests - #280
vidyalakshmir wants to merge 3 commits into
Conversation
|
Thanks for the work. I’ll start on the apps pipeline using this. I just confirmed my understanding. Are 11 apps (awk, bash, coreutils, curl, git, grep, lmbench, make, nginx, perl, sed) standardized, while cpython and tinycc are excluded? Additionally, to run make test for all 11 apps, I think we should also update TESTABLE_APPS in the Makefile |
|
Yes, need to update TESTABLE_APPS in the Makefile There are two categories of test suites for apps :
Adapting test suite of apps to run with lind-wasm requires some porting effort. Hence, we have We have Cpython and coreutils (from category 1) and tinycc pending. Regarding postgres, I am not completely sure about which category its tests are. @rishabhBudhouliya Do you have more information about postgres test suites. |
Just curious, what is the reason for dividing the tests into Category 1 and 2? |
Adapting test suite of apps to run with lind-wasm requires some porting effort. Depends on if the app comes with test suite and the porting effort. |
In that case, is the ideal approach to test both Category 1 and 2? It looks like Coreutils is listed under both categories. Or is it fine to just run either Category 1 or Category 2? |
postgres is runnings its own regression testsuite |
|
@vidyalakshmir As Qianxi mentioned, Postgres has its own test suite called pg_regress. The usual way to run it is to spawn a postgres server and then run the pg_regress harness which itself spawns a psql client to talk to the main server and then run a bunch of tests. |
rishabhBudhouliya
left a comment
There was a problem hiding this comment.
Thanks Vidya for leading the standardization effort. I mostly have app specific behavior related comments. Please let me know how I can help.
Other app related behavior I saw:
Difference in how PASS/FAIL is represented in different app tests results:
[PASS]/[FAIL] is PASS:/FAIL: in curl, git, nginx, lmbench, sed
There was a problem hiding this comment.
I suspect nearly every test name contains spaces. Should we use a newline/ pipe delimeter?
There was a problem hiding this comment.
Yeah makes sense
There was a problem hiding this comment.
It seems like some timeout configs are inconsistent:
- awk/make/perl/bash/coreutils hardcode TIMEOUT_SECS=N;
- sed/grep/curl/git use ${TIMEOUT_SECS:-N}.
| echo "================================" | ||
|
|
||
| if [ "$FAIL" -gt 0 ]; then | ||
| exit 1 |
There was a problem hiding this comment.
Since this exits 1 on failure, with Makefile .SHELLFLAGS := -eu -o pipefail -c + .ONESHELL, a coreutils failure aborts the entire make test run. Is that the intended behavior?
There was a problem hiding this comment.
No, even if one test fails, need to continue to the next test.
|
@rishabhBudhouliya, @qianxichen233 I was running 'make test' on the First, about running the tests at all. The current test:
@rc=0; \
for app in $(APP); do \
case " $(TESTABLE_APPS) " in \
*" $$app "*) ;; \
*) echo "ERROR: unsupported test app '$$app'"; exit 1 ;; \
esac; \
if [[ -x '$(APPS_ROOT)/'"$$app"'/run_tests.sh' ]]; then \
if ! '$(APPS_ROOT)/'"$$app"'/run_tests.sh' "$$app"; then \
rc=1; \
echo "[FAIL] $$app"; \
fi; \
else \
echo "[SKIP] $$app: missing run_tests.sh"; \
fi; \
done; \
exit $$rcWrapping the call in a conditional keeps This a result of 'make test' with the change.
That's more failures than I expected, so I'd appreciate it if you could point out anything I might have missed. Image:
|
Thanks @Uk-jake You could make necessary changes to Makefile to adapt it to the CI/CD pipeline. Also, thanks for running the test suites. It would also help if you could attach or post the errors you face for the failed apps. |
|
So I tried testing some of the apps (curl) which had failures. And seems like the issue is mostly with |
f7339f7 to
8b16e83
Compare
|
@vidyalakshmir @qianxichen233 have created a postgres PR for app test standardization: #282 |
Thanks @rishabhBudhouliya |
|
@Uk-jake After running the tests, the two cases worth noting are :
|
|
@vidyalakshmir, @rishabhBudhouliya |
|
@Uk-jake I am not 100% confident but these apps don't require fpcast |
TESTING.mdwhich specifies the app test requirementsscripts/test_lib.shwhich is a common script to skip tests