Skip to content

Disable tests reruns on dev profile#1149

Closed
ivankelly wants to merge 4 commits into
apache:masterfrom
ivankelly:dev-rerun
Closed

Disable tests reruns on dev profile#1149
ivankelly wants to merge 4 commits into
apache:masterfrom
ivankelly:dev-rerun

Conversation

@ivankelly
Copy link
Copy Markdown
Contributor

Sometimes you don't want the rerun because you want to see the flake.

Sometimes you don't want the rerun because you _want_ to see the flake.
@ivankelly ivankelly self-assigned this Feb 15, 2018
@ivankelly ivankelly requested review from eolivelli and sijie February 15, 2018 15:43
@sijie
Copy link
Copy Markdown
Member

sijie commented Feb 15, 2018

this pull request says "sometimes you don't want the rerun because you want to see the flake". then what about some other time or what about people who want the settings in the dev profile, but they want "rerun". I don't think this PR or even those dev/debug profiles belong to the code base itself. because they are kind of very subjective and personal profiles, which it is not used by any CI pipeline.

I believed I put my points on slack when those dev profiles were introduced. I would like to raise my points again here, since I started to see there are a few changes about dev profiles recently. Having a "dev" profile is good for development, but it is kind of too personal. It can be achieved by defining a bash alias on mvn command with overrided system properties, rather than defining two dev profiles in the code base, which only used by a few people.

@ivankelly
Copy link
Copy Markdown
Contributor Author

There are certain things in the dev profile which cannot be overridden at the commandline, like redirectTestOutputToFile.

I've just tested "-Dsurefire.rerunFailingTestsCount=0" also, and it doesn't seem to have any effect.

@sijie
Copy link
Copy Markdown
Member

sijie commented Feb 15, 2018

There are certain things in the dev profile which cannot be overridden at the commandline, like redirectTestOutputToFile.

I think you can achieve with proper system property settings. but I am not sure, need to take a look into it.

I've just tested "-Dsurefire.rerunFailingTestsCount=0" also, and it doesn't seem to have any effect.

I think it is because the setting was set in the pom file. if we don't set it in pom file, we can use the system property setting.

anyway, we can look into how to improve this. not going to block this PR if you need this change. my intention here is to make sure my concerns I raised before and here will be kept in our mind when we are making changes to those dev profiles.

@ivankelly
Copy link
Copy Markdown
Contributor Author

I think you can achieve with proper system property settings. but I am not sure, need to take a look into it.

I have tried many times. If you can find I way I would be very pleased to see how, but a profile is the only way I've managed to make it work thus far.

anyway, we can look into how to improve this. not going to block this PR if you need this change. my intention here is to make sure my concerns I raised before and here will be kept in our mind when we are making changes to those dev profiles.

I don't think it's something that merits much time being spent on it. To me, the developers are the primary consumers of the codebase, so anything that makes it easier for them(and me, being a developer) to work with it is good.

This dev profile is very useful for me. I regularly use disable the redirectToFile for debugging, and working with the integration tests, the reruns become a pain, because everything after the first run fails because of dirty environment. As I said above, if you can find a better way without the profile, I'm open to that, but everything else i've tried fails.

@sijie
Copy link
Copy Markdown
Member

sijie commented Feb 16, 2018

I understand what you want. And as I said, I am not blocking this change here, since I don’t really have cycles to get into this.

I agree developers are the consumers of the codebase. But my concern is the change here and the comments are indicating these changes are suitable only for some developers but not all. For example, I don’t use dev profile and i feel redirect to output files is much better because I can use all the Linux tools to search in the file.

Also to be clear, I am fine with a common profile in the code base for making developer easier. What I am concerning here about the PR is we are adjusting a profile based on one developer feel inconvenient about the settings. It ends up becoming a profile that is only used by one or two people and the remaining of the community doesn’t really use it.

Again these are only my concerns. Other people can think differently. As I said, I am not going to block this change if you guys feel you need this change to be merged, go for it.

@ivankelly
Copy link
Copy Markdown
Contributor Author

Also to be clear, I am fine with a common profile in the code base for making developer easier. What I am concerning here about the PR is we are adjusting a profile based on one developer feel inconvenient about the settings. It ends up becoming a profile that is only used by one or two people and the remaining of the community doesn’t really use it.

If this becomes a problem, we could have a profile per setting.

@ivankelly
Copy link
Copy Markdown
Contributor Author

retest this please

@asfgit
Copy link
Copy Markdown

asfgit commented Feb 16, 2018

Can one of the admins verify this patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants