Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues related to test scripts #458

Closed
supercooldave opened this issue May 26, 2016 · 19 comments
Closed

Issues related to test scripts #458

supercooldave opened this issue May 26, 2016 · 19 comments
Assignees

Comments

@supercooldave
Copy link

supercooldave commented May 26, 2016

Issues testing framework:

  • calls Module system three times
  • makefile in typesyn directory can be removed
  • when the script encounters a .enc file without a corresponding .out file, it simply prints the name of the file, and continues. It does not run treat it as a test, which is fine, but it does not say what it has done with the file. The output is exactly the same as for a test that runs and succeeds.
  • if I do something like
bin/test encore/rubbish

where there is no such thing as rubbish, I get

ALL TESTS PASSED
cat: //tmp/test_results_gVySmh/test_counts.log: No such file or directory

It would be preferable to have it say that no tests were found.

  • The test script outputs
rm: encore/modules/Lib: is a directory

in the middle of testing.

  • I would really prefer a local DISABLED_TESTS.grep file, so that I don't have to look up 2 directories and tamper with a file that others might be tampering with. It makes more sense to me.
  • an empty .fail file causes a test to pass, irregardless of the actual status. This case seems wrong -- error-prone, and should be detected.
  • actually fail case seem not to work at all.
  • why does it say: >>> FAILED TEST: Qualified.FAILED: in the test output.
    Specifically, why .FAILED not .enc?
  • I can't seem to run individual tests -- I have a test encore/modules/InfiniteImport.enc that I want to run. But this doesn't work:
dhcp-11-160:tests dave$ bin/test encore/modules/Infinit
ALL TESTS PASSED
cat: //tmp/test_results_L3hkZU/test_counts.log: No such file or directory
dhcp-11-160:tests dave$ 

Actually, it is not clear whether or not the test has been run.

  • How can I pass flags (such as library paths) to the compiler?
  • Previous version of make test cleaned up and remade the compiler before running the tests. This behaviour should be preserved.
@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

actually fail case seem not to work at all.

I don't know what you mean by that. Have you read the documentation in README.md?
Edit: I think I found out myself.

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

How can I pass flags (such as library paths) to the compiler?

Using a Makefile. The script will call make if it finds a Makefile.

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

make test does still build the compiler. I haven't touched it. https://github.com/parapluu/encore/blob/development/Makefile#L24

@supercooldave
Copy link
Author

How can I pass flags (such as library paths) to the compiler?
Using a Makefile. The script will call make if it finds a Makefile.

I don't find this an adequate solution. If one test needs to be different (that is, use a compiler flag), then I need to write a Makefile for all of the tests in that directory. This kind of defeats the purpose of having the testing script.

My suggestion is to add a .flags file containing flags to pass to the compiler.

@supercooldave
Copy link
Author

I read the documentation for .fail. Here is what I witnessed:

  • An empty .fail file causes test to pass -- technically, this satisfies the specification you have given, but it is unexpected, so the tester should be warned.
  • A .fail file containing foo passed the test even though foo did not appear in the output of the compiler. This is not what I expected.

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

I read the documentation for .fail. Here is what I witnessed:

I had already edited my original question. I already have a local fix for that.

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

then I need to write a Makefile for all of the tests in that directory.

You don't need to write a Makefile for all of the tests. Just for the one. Makefiles are what was there before and I chose to reuse them in the interest of keeping this task manageable. A flags file would have the disadvantage of people having to learn about it first, it would also be less powerful (you can do more things using make).

That being said, a flags file would not be a horrible solution. Just more work, and brittle.

@supercooldave
Copy link
Author

Firstly, it is good that I can used the Makefile and other individual tests.

Secondly, I have a Makefile of 31 lines that I need to maintain just so that -I lib:otherlib is passed in to the compiler to test certain aspects of the module system. I don't need all the flexibility of make, and it is totally reasonable to want to pass flags into the compiler.

I don't want to make this an onerous task. I just want one coherent test framework. Mostly, you have produced such a thing. It is only now when I test it out that I see what features I need that I didn't realise I needed.

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

Can I see that Makefile?

@supercooldave
Copy link
Author

ROOT_PATH=../../../..
ENCOREC=$(ROOT_PATH)/release/encorec

#INGORE=Lib.enc

#ENCORE_SOURCES=$(shell ls *.enc)
ENCORE_SOURCES=DeepImporter.enc FarImporter.enc DupImporter.enc Qualified.enc

ENCORE_TARGETS=$(ENCORE_SOURCES:.enc=)

build:
    @$(foreach PROG,$(ENCORE_TARGETS), make -Bqk $(PROG) || make -Bk $(PROG) || true;)


test: build
    @echo
    @echo Module tests:
    @./test.sh $(ENCORE_TARGETS)


%: %.enc
#ls -l
    @rm -f $@
    @echo "compiling '$@'"
    @$(ENCOREC) [email protected] -clang -c -I lib:otherlib

clean:
    @rm -rf *.pony.c *.dSYM *_src $(ENCORE_TARGETS)

.PHONY: test clean

@supercooldave
Copy link
Author

Compare to the .flags file:

-I lib:otherlib

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

5 lines. With the flag file, you'd have 4 files with one line each.

ENCOREC=../../../../release/encorec

build: DeepImporter FarImporter DupImporter Qualified

%: %.enc
    $(ENCOREC) [email protected] -clang -c -I lib:otherlib

.PHONY: build

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

Anyway, I'll take care of the show stoppers that you've found and delegate the Makefile question to a new issue.

@supercooldave
Copy link
Author

Can you indicate which ones you do take care of, and I will create an issue for each of the others that remains a problem?

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

Yes, I'll do that with the PR that I should be ready with in the next hour or so.

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

This discussion led me to look closer at the Makefiles. Most can go :) Will be included in the next PR.

@supercooldave
Copy link
Author

It would be interesting to understand why the Makefiles that need to stay need to stay.

@kaeluka
Copy link
Contributor

kaeluka commented May 27, 2016

The one in modules is the only one left. It uses the -I flag.

@supercooldave
Copy link
Author

Fixed in #463.
New issue will be created for some remaining problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants