search

AnyDSL / impala

An imperative and functional programming language
https://anydsl.github.io
GNU General Public License v3.0
151 stars 12 forks source link

Fixing type inference tests #48

Open madmann91 opened 7 years ago

madmann91 commented 7 years ago

Many tests for impala are broken because we currently print types such as fn(i32) -> i32 as fn(i32, fn(i32)). The code that did the pretty printing previously is now commented out in src/impala/sema/type.cpp. Should we change the tests and get a rid of the commented code, or should we re-enable the pretty printing ?

leissa commented 7 years ago

We should fix pretty printing.

That being said, most sema tests are broken anyway since they have not been ported after merging the infer branch. Too many things changed...

If you want to update the sema tests, the best way would be:

stlemme commented 7 years ago

Beside fixing tests, I suggest to move all tests into an AnyDSL/testsuite.git repo and ensure, that all tests ported there are known to pass. This would ease to automate testing.

leissa commented 7 years ago

IMHO tests belong to the stuff they are testing. Most tests in impala/tests are actually tests for impala. However, there are a couple of tests which actually test things in thorin. Unfortunately, we haven't an infrastructure in place to test thorin without impala.

What advantages would you have from a separate testing repo? I only see disadvantages: For example, you have to pair the testing repo revision with the corrent impala repo revision.

stlemme commented 7 years ago

Mainly, because it doesn't matter which IMPALA_BIN you use for testing. You could even use the docker image of anydsl/impala. Moreover, you get rid of the outdated stuff and ensure the testsuite always works against impala@master.

Since there are already tests for thorin, you would actually need to pair thorin and impala today. However, we do not enforce that.

I would even expect more tests (i.e. integration tests rather than unit tests) for instance to validate the runtime works correctly.

leissa commented 7 years ago

I think we should put this discussion offline, and discuss that on our next meeting - we're getting a bit off-topic :)