The Wayback Machine - https://web.archive.org/web/20201113100824/https://github.com/ethereum/solidity/issues/6432
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

Refactor interactive tests #6432

Closed
chriseth opened this issue Apr 1, 2019 · 12 comments
Closed

Refactor interactive tests #6432

chriseth opened this issue Apr 1, 2019 · 12 comments

Comments

@chriseth
Copy link
Contributor

@chriseth chriseth commented Apr 1, 2019

Many interactive tests share common functionality and functionslike printIndented. They should be pulled into a common base class.

Also, a "simple" interactive test that just has an input and an output (and comparison is string comparison) could profit from a simple combined base class.

@chriseth chriseth added this to To do in 0.5.8 via automation Apr 1, 2019
@ekpyron
Copy link
Member

@ekpyron ekpyron commented Apr 5, 2019

It might even make sense to combine this with allowing multiple expectation blocks per test already - then a common class would split the input into source and multiple expectation blocks, determine which kind of test each expectation block is (and parse common settings) and then let the specific test case handle the rest.

@chriseth
Copy link
Contributor Author

@chriseth chriseth commented Apr 5, 2019

@ekpyron I think we should first clean up the mess and then make it more complicated again :)

@ekpyron
Copy link
Member

@ekpyron ekpyron commented Apr 5, 2019

I think really cleaning it up would basically mean rewriting quite some parts of it and then one might as well split into multiple expectation blocks right away, since otherwise we'll probably rewrite things once more later on anyways :-). But yeah - both would be fine and I would say whoever actually does this can decide which seems better and easier.

@chriseth chriseth removed this from To do in 0.5.8 Apr 30, 2019
@chriseth chriseth added this to To do in 0.5.9 via automation Apr 30, 2019
@chriseth chriseth removed this from To do in 0.5.9 May 27, 2019
@chriseth chriseth added this to To do in 0.5.10 via automation May 27, 2019
@chriseth chriseth moved this from To do to Ready to be worked on in 0.5.10 May 29, 2019
@chriseth chriseth added this to Inbox in Backlog (non-breaking) via automation Jun 26, 2019
@chriseth chriseth removed this from Ready to be worked on in 0.5.10 Jun 26, 2019
@maharsh312
Copy link

@maharsh312 maharsh312 commented Jul 7, 2019

I would like to work on it. I have some experience in C++. This seems to look like something interesting to work on.

@erak
Copy link
Collaborator

@erak erak commented Jul 8, 2019

@maharsh312 That sounds great! Do you need some guidance on where to start or a more detailed explanation of what has been discussed in this issue? Just let us know what's needed to get you started on this. We're more than happy to help!

@chriseth
Copy link
Contributor Author

@chriseth chriseth commented Jul 8, 2019

One piece of code you could look out for is

	if (m_expectation != m_obtainedResult)
	{
		string nextIndentLevel = _linePrefix + "  ";
		AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Expected result:" << endl;
		printIndented(_stream, m_expectation, nextIndentLevel);
		AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::CYAN}) << _linePrefix << "Obtained result:" << endl;
		printIndented(_stream, m_obtainedResult, nextIndentLevel);
		return TestResult::Failure;
	}

This is shared in quite some files. Especially if m_expectation and m_obtainedResult is a string, it can be pulled into a common base class, which could be named something like SimpleTestCase that itself inherits from TestCase.

@maharsh312
Copy link

@maharsh312 maharsh312 commented Jul 9, 2019

I think I don't need help as of now. Do you guys have an IRC?

@maharsh312
Copy link

@maharsh312 maharsh312 commented Jul 9, 2019

I think I don't need help as of now. Do you guys have an IRC?

Never Mind. Found It!

@kennycastro007
Copy link

@kennycastro007 kennycastro007 commented Aug 12, 2019

Is this issue still open? I'm trying to find good first issues to start contributing to open source.

@ekpyron
Copy link
Member

@ekpyron ekpyron commented Aug 12, 2019

@kennycastro007 We thought this one might be a good issue for you to start on, if you like. If you need help you can talk to us in our gitter channel https://gitter.im/ethereum/solidity-dev. When you start working on this, please assign the issue to you.

@Marenz
Copy link
Contributor

@Marenz Marenz commented Aug 12, 2019

I believe ppl not part of the team can't assign issues. Just let us now if that is the case and we'll assign it for you.

@chriseth
Copy link
Contributor Author

@chriseth chriseth commented Jun 2, 2020

I think after #9059 this looks good now!

@chriseth chriseth closed this Jun 2, 2020
Backlog (non-breaking) automation moved this from Inbox to Done Jun 2, 2020
Reducing Technical Debt automation moved this from To do to Done Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.