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

Abort the job on the main thread if the test suite fails #27

Closed
TheRustifyer opened this issue Nov 3, 2023 · 4 comments
Closed

Abort the job on the main thread if the test suite fails #27

TheRustifyer opened this issue Nov 3, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request test-suite

Comments

@TheRustifyer
Copy link
Member

Our new test-suite is mostly working as expected, which is extremely fine, but we need to level it up.

Now, the test suite runs all the tests, and just show the results. But the test-suites typically panics or throws when a test fail.
And for that reason, we need to implement such change in our test suite.

Think, for example, in a typical Github pipeline. If anything fails, the action will conclude successfully, even there are wrong tests.

The task is simply. But is based in one decision. Fail fast or no.
The thing is that the user, when calls the RUN_TESTS() function, it should be able to pass as a parameter a configuration that allows to configure the fail-fast setting.

This means, by default, when the test-suite runs a failed test, it should abort the main thread, leaving the job and raising an exception. But the user should be able to specify when it prefers to run all the tests, and then, at the end, in the results, see what tests are failing (instead of throwing in the first non passed test).

For now, this can be acomplished with a simple boolean, that by default is false, as a parameter of the before mentioned function.

@gbm25
Copy link
Member

gbm25 commented Nov 26, 2023

As we have discussed several times in recent weeks, and for the record in the issue, we are going to postpone being able to control this behavior by command line, because implementing all that this implies is an order of magnitude above what we need right now.

Today I have been able to give a good push to this problem, and at the moment I have an implementation based on an enum with 3 behaviors:

  • Run all tests.
  • In case of error in a suite/freetest it stops that suite/freetest and goes to the next one.
  • In case of failure in a suite/freetest for all and does not run the following tests.

I am still checking that everything works correctly, as soon as I validate it I will change parts of the code that are not very readable or can be improved and I will add the necessary output and documentation.

@gbm25
Copy link
Member

gbm25 commented Dec 9, 2023

A quick update on this issue.

I just did a couple of changes and tests, everything seems to be working correctly.
As we discussed, an exception is now thrown in case there is an error in a test, regardless of the mode being used, the only difference is when this exception is thrown.

I would like to review the changes with you @Pyzyryab to be sure that everything is as you had intended and to see if I should implement the exception in a different way, because I'm not sure if it is exactly what you were looking for and I do not have enough knowledge to know if it makes sense as I did or should be done differently.

On the other hand the code could be improved, like grouping the outputs, and changing the colors using the stylizer, but it would be better to do it another time to be able to move on to something else.

@TheRustifyer
Copy link
Member Author

@gbm25 yes, I agree. In #32 you can focus only on do the job that we've discussed, and I will be glad to review with you the changes when you're ready.
Later, we can open a different Pull Request to clean and upgrade the code if you fell more comfortable with that approach, or we can try to include them in this one.
We can discuss the better approach when we meet.

@TheRustifyer
Copy link
Member Author

Closed because of #32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request test-suite
Projects
None yet
Development

No branches or pull requests

2 participants