Undo-redo support on Test Plan tree modification #1930
Comments
|
Michel Nolard (migrated from Bugzilla): If the undo-redo support is not implemented/finalized/integrated to the next |
|
Sebb (migrated from Bugzilla): Since Control-R is used for a normal run, I've changed Run Remote to Control- This is in SVN r542707. Also fixed the confusing "Bad call" messages in r542633. |
|
Taimo Peelo (migrated from Bugzilla): Lacking undo/redo is bad and frustrating nuisance. Just one bad edit, forgetting the previous contents of some obscure request parameter... what then? |
|
Sebb (migrated from Bugzilla): If anyone can provide a patch, then of course it will be evaluated and applied if appropriate. |
|
immanuel.hayden (migrated from Bugzilla): Just wanted to express my support for addressing this issue. I would love to see undo/redo support for two independent areas:
Many thanks for listening :) |
|
Andrey Pokhilko (migrated from Bugzilla):
Created attachment undo7 - alternative path recording.patch: proposed patch |
|
Sebb (migrated from Bugzilla): Just a couple of minor points: We release code under the Apache License and each file has to have the standard AL header. These are missing from the new files you have provided. Also, we don't use @author tags in code. This is mainly because the ASF is about community rather than individuals; also the tags are impossible to maintain accurately as code is further developed. Instead, we credit contributors in the changes list and elsewhere (e.g. Wiki) So please could you either confirm that it is OK for us to add AL headers and remove @author tags, or provide an updated patch? Thanks! |
|
Andrey Pokhilko (migrated from Bugzilla): Of course it would be nice to be mentioned in changelog or somewhere else, it is left for your choice. I just glad to be reason for closing 5-year old issue :) |
|
@pmouawad (migrated from Bugzilla):
So I think patch need more work. Thanks for it anyway and we will try to find some solutions. |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla):
Regards Created attachment BUG_42248.patch: Updated Patch |
|
@pmouawad (migrated from Bugzilla):
Also note that patch is on r1231672 |
|
Andrey Pokhilko (migrated from Bugzilla): One question: have youn noticed my notes on selecting historical path in tree? Is it possible in JMeter? |
|
@pmouawad (migrated from Bugzilla):
|
|
Andrey Pokhilko (migrated from Bugzilla): |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
Andrey Pokhilko (migrated from Bugzilla): Created attachment undo_improved.patch: Patch for next attempt of undo |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla): Do you confirm that patch only adresses undo-redo on test tree ? |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla): Is it possible to only create 1 patch file containing everything ? |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla): If you want you can also provide a pull request It will allow me to review it and give you feedback very soon, otherwise you will have to wait for 21th august. Regards |
|
@pmouawad (migrated from Bugzilla): |
|
Andrey Pokhilko (migrated from Bugzilla): Created attachment undo_improved-files_added.patch: patch with added files |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla):
|
|
Andrey Pokhilko (migrated from Bugzilla):
The undo for TestElement property change already works. History is recorded once changes applied to TestPlan. I think disabling the undo is unnecessary, since it is vital feature for JMeter users. The option to limit history size helps protecting from too much memory consumption. The choice seems to be to provide people undo without in-fields undo, or not provide ability to revert the actions at all. I believe the value for this feature is too high to block it for more years waiting to implement in-fields undo... Hope this makes sense. Created attachment undo_improved-files_added.patch: proposed patch with more changes |
|
shmulikk (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla): @Shmullik, see @jmeter_plugins twitter account for info on this. Note that once we integrate this in nightly build, we will require test help from users community, so your tests will be welcome. |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla): Thanks |
|
Andrey Pokhilko (migrated from Bugzilla): |
|
Sebb (migrated from Bugzilla):
Looks like this is a Bugzilla issue. Diff / Raw Unified You need to click on the attachment name, or use Details. |
|
@pmouawad (migrated from Bugzilla): URL: http://svn.apache.org/r1622936 Added: |
|
@pmouawad (migrated from Bugzilla): URL: http://svn.apache.org/r1622938 Modified: Date: Sat Sep 6 21:23:48 2014 URL: http://svn.apache.org/r1622939 Modified: Date: Sat Sep 6 21:34:45 2014 URL: http://svn.apache.org/r1622941 Modified: |
|
@pmouawad (migrated from Bugzilla):
Many thanks for this contribution ! |
|
@pmouawad (migrated from Bugzilla): URL: http://svn.apache.org/r1622945 Modified: |
|
@pmouawad (migrated from Bugzilla): |
|
@pmouawad (migrated from Bugzilla): URL: http://svn.apache.org/r1622947 Modified: |
|
Andrey Pokhilko (migrated from Bugzilla): Regarding toolbar buttons enabling-disabling there is a problem, since JMeter code does not offer interface for enabling/disabling icons easily, at leas I did not find one. So I left it enabled and put a TODO comment for this. I have dowloaded latest code and now toolbar buttons are always disabled. I suppose this was not your intention. From what I've seen in the code, implementing the interface for proper buttons enabling/disabling would require some work, that's why I did not try to solve it together with Undo, just don't want to link two big problems. |
|
@pmouawad (migrated from Bugzilla): I will fix this this afternook hopefully, I disabled it temporarily in trunk. Thanks for noticing and reporting it. |
|
@pmouawad (migrated from Bugzilla): URL: http://svn.apache.org/r1622984 Modified: Date: Sun Sep 7 14:04:34 2014 URL: http://svn.apache.org/r1623019 Modified: |
|
UbikLoadPack support (migrated from Bugzilla):
Analysing it, is it due to JMeterTreeNode#setMarkedBySearch calling treeModel.nodeChanged(this). Regards |
|
UbikLoadPack support (migrated from Bugzilla): Regards Created attachment BUG_42248.patch: Patch that disables Undo/Redo feature if undo.history.size is set to 0 BUG_42248.patchIndex: src/core/org/apache/jmeter/gui/GuiPackage.java
===================================================================
--- src/core/org/apache/jmeter/gui/GuiPackage.java (revision 1624847)
+++ src/core/org/apache/jmeter/gui/GuiPackage.java (working copy)
@@ -137,7 +137,9 @@
*/
private GuiPackage(JMeterTreeModel treeModel, JMeterTreeListener treeListener) {
this.treeModel = treeModel;
- this.treeModel.addTreeModelListener(undoHistory);
+ if(undoHistory.isEnabled()) {
+ this.treeModel.addTreeModelListener(undoHistory);
+ }
this.treeListener = treeListener;
}
@@ -156,7 +158,9 @@
* - Locale Changes
*/
public void registerAsListener() {
- this.undoHistory.registerHistoryListener(this);
+ if(undoHistory.isEnabled()) {
+ this.undoHistory.registerHistoryListener(this);
+ }
JMeterUtils.addLocaleChangeListener(this);
}
Index: src/core/org/apache/jmeter/gui/UndoHistory.java
===================================================================
--- src/core/org/apache/jmeter/gui/UndoHistory.java (revision 1624847)
+++ src/core/org/apache/jmeter/gui/UndoHistory.java (working copy)
@@ -133,6 +133,11 @@
* @param comment String
*/
public void add(JMeterTreeModel treeModel, String comment) {
+ if(!isEnabled()) {
+ log.debug("undo.history.size is set to 0, undo/redo feature is disabled");
+ return;
+ }
+
// don't add element if we are in the middle of undo/redo or a big loading
if (working) {
log.debug("Not adding history because of noop");
@@ -338,6 +343,14 @@
}
/**
+ *
+ * @return true if history is enabled
+ */
+ boolean isEnabled() {
+ return HISTORY_SIZE > 0;
+ }
+
+ /**
* Register HistoryListener
* @param listener
*/ |
|
shmulikk (migrated from Bugzilla): I think that this undo-redo is a major milestone and it will require some more effort to make it public due to the current quality, which is not enough. It might make sense to close this ticket/bug - and have all follow-ups in separate bugs. |
|
@pmouawad (migrated from Bugzilla): Modified: |
|
Chaitanya Bhatt (migrated from Bugzilla):
It does not look like JMeterTreeNode#setMarkedBySearch gets called when the project is loaded. So the 60 second delay in loading a 2MB project does not appear to be caused by setMarkedBySearch function. I am working on finding the root cause for the delay during the project loading phase. |
|
Chaitanya Bhatt (migrated from Bugzilla):
Philippe, It looks like Save#convertSubTree is the culprit for slowing down the clone operation. Any idea why this blow code is slow? void convertSubTree(HashTree tree) { |
Michel Nolard (Bug 42248):
Hi !
I am very happy with JMeter, which is very useful and powerful ! So, I decided
to contribute in my way by pointing those little annoying things which the users
had to put up with but didn't notice or didn't take the time to report.
My thought are that a Undo/Redo feature is required while editing a test plan as
it is well known that user do mistakes and don't want to be punished simply
for being human.
At first, I would argue that adding such a framework to JMeter would imply some
work, refactoring some things and implementing Command Pattern to ensure proper
efficiency and power. When I think a little bit more about it, I feel it to be
like an "internal XML patching system", as the problem is to apply modifications
to the plan test tree (which can be represented as an XML tree) and add more
modifications or revert some others.
I hope this won't be too hard to implement in JMeter. I simply don't know as
I've only thought about it but not designed code to do it (which I am not able
to write at this time).
Votes in Bugzilla: 11
OS: All
Duplicates:
Depends on:
The text was updated successfully, but these errors were encountered: