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

CASSANDRA-16012 sstablesplit tool unit testing #762

Closed
wants to merge 1 commit into from

Conversation

bereng
Copy link
Contributor

@bereng bereng commented Oct 1, 2020

No description provided.

@bereng
Copy link
Contributor Author

bereng commented Oct 1, 2020

CI

@bereng
Copy link
Contributor Author

bereng commented Oct 7, 2020

CI j11
CI j8

@bereng
Copy link
Contributor Author

bereng commented Oct 8, 2020

Well that was a surprise @yifan-c The new custom junit executor was wrong. Adding that new test method you suggested I noticed I couldn't get any failures. The tests were green despite I knew they were broken. I found out the new executor was swallowing all errors. So I removed that and overrode the @After method, which on retrospect I don't know how I didn't think of it before.

Now everything works as expected, it's cleaner and has the extra test method.

CI j11
CI j8

Copy link
Contributor

@yifan-c yifan-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Like the overriding approach better. It's cleaner.

So.. in order to make the custom runner correctly ignores After or so and not swallow errors, the override need to just return the input statement. In the super class, the runner detects if after exists and run.

        @Override
        protected Statement withAfters(FrameworkMethod method, Object target, Statement statement)
        {
            return statement;
        }

@bereng
Copy link
Contributor Author

bereng commented Oct 8, 2020

Latest review comments pushed. Thx.

@bereng
Copy link
Contributor Author

bereng commented Oct 9, 2020

CI for latest force push here

@bereng bereng closed this Nov 5, 2020
@bereng bereng deleted the CASSANDRA-16012 branch November 5, 2020 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants