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

Results reporting is not useful #68

Closed
chick opened this issue Sep 17, 2019 · 5 comments
Closed

Results reporting is not useful #68

chick opened this issue Sep 17, 2019 · 5 comments
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@chick
Copy link
Contributor

chick commented Sep 17, 2019

The treadle backend calls the report method which has a field for number of tests (i.e. expects) that succeeded. But because testers2 does not use treadle's expect this number is always zero. This is confusing to new users. Simple fix would be to remove the report call, but I think it would be warmer and fuzzier to give some actual stats.
This would be nice for other backends, so it should probably be handled at the testers2 level and not within the specific backend.

@chick chick added bug Something isn't working invalid This doesn't seem right labels Sep 17, 2019
@ducky64
Copy link
Member

ducky64 commented Sep 17, 2019

According to the comments in TreadleBackend, report is called to dump VCDs. Is there another treadle API to do the same thing / cleanly close the backend? I think the reporting is purely a side effect.

My own preference is that unit tests shouldn't print anything except for a pass / fail unless the user enables debugging flags - automation is the goal and needing to read through the console dump defeats that purpose...

@chick
Copy link
Contributor Author

chick commented Sep 17, 2019

Yes, the alternative is to call tester.finish in TreadleBackend instead, as it will just flush the vcd and print nothing. So that should probably be done.

OTOH, While I agree with you that automation is a big goal, the other no less important one is to enable test driven development, which is non-automated and usually involves a lot of looking at the tests outputs. To emphasize this, testers2 will also be more and more a part of the bootcamp. I think users will be better served to see some obvious success message rather than the unixy
Process finished with exit code 0

@seldridge
Copy link
Member

I share @ducky64's preference here. I'd like to see the output as indistinguishable from scalatest. The only default reporting should be limited to standard scalatest testing information, scalatest methods (info, scenario, feature, etc.), and user-explicit printf. (I'd go one step further and turn off all the Verilator compilation junk, too.)

@ducky64
Copy link
Member

ducky64 commented Sep 18, 2019

I think the ScalaTest framework test(...) should not print anything by default at the very least.
However, it might be useful to think of how we could allow the user to print additional debugging data - maybe test() should return a status object that could be directly printed to the console? Maybe RawTester should have a test() variant that has an integrated print, or returns useful directly-printable status?

@seldridge
Copy link
Member

I think that info currently doesn't print anything if used in testers2. It would be nice if this was supported as it seems to be the most canonical way to print additional information during a test. I use this pretty heavily in some of the newer FIRRTL testing and it makes the tests look pretty clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants