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

Feature/disable test history #1494

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AlexNetcare
Copy link
Contributor

Adding a new Property on a page to disable Test Histories for that page. This is turned off by default.
This might be taking care of this Request #1488
The new Property does not work recursive though.

@six42
Copy link
Contributor

six42 commented Apr 3, 2024

Hi Alex,

Are you aware of the existing nohistory parameter. This works recursive.
https://fitnesse.org/FitNesse/UserGuide/AdministeringFitNesse/RestfulServices.html

@AlexNetcare
Copy link
Contributor Author

Hi @six42,
I actually was not aware of that parameter. But after looking what it does, it is not really usable for our Testers.
The idea of my change was, that you could disable the Test History of a single page. So that when you run a suite, those disabled Tests do not generate a Test History. That is because some of our Tests only generate informative stuff which does not need to have a History.
Also in our case, our Testers execute Tests manually most of the time. So when they press on "Suite" or "Test" the nohistory parameter would not be added.
I also saw that my logic didn't do anything for Suites. So I added some logic, that when the property to disable Test Histories is set on a Suite, it will follow the same rule as the nohistory parameter.

@six42
Copy link
Contributor

six42 commented Apr 17, 2024

Would you like the feature to work recursively?

I am wondering if this feature should be configured via a page property as you did or rather by a variable.
Most parameters which steer the test system are defined as variables like

!define slim
!define slim.port {1}

Variables have the advantage that they can be defined in many ways:

  • via plugin.properties for the whole wiki,
  • as parameters to a single REST request
  • or on a page with define.

They always work recursively.

Page properties so far define the page security and the menu layout, the type of page and the source of the data.

What do others think when should be something be configured via a property and when via a variable?
Would be good to have some guiding rules.

@AlexNetcare
Copy link
Contributor Author

Sorry for the late answer but I had some discussions with colleagues about this. To cut it short, from our side we would like to keep it as a page property for now.
We don't really want this variable to be used on sub pages since some of them should still generate test histories.
But I would also appreciate some more input from others. If changing the page properties here is the wrong place, then I would change to page variables and see if that works for our Testers.

@AlexNetcare
Copy link
Contributor Author

This has been a while now. Is it possible to get any new Input on this?

@fhoeben
Copy link
Collaborator

fhoeben commented Oct 23, 2024

@six42 any further thoughts?

@AlexNetcare
Copy link
Contributor Author

@fhoeben Would this change be problematic to merge? Right now it would just be an option do disable the test history for manual testing. If there will be an Use Case in the future for recursive properties maybe then it would make sense to to create a new PR and either change to page variables or to make the page property recursive.

@fhoeben
Copy link
Collaborator

fhoeben commented Dec 5, 2024

Can you rebase to resolve all conflicts?

@fhoeben
Copy link
Collaborator

fhoeben commented Dec 17, 2024

@AlexNetcare when I look at the 'Files changed' overview of this PR I believe something went wrong rebasing/merging your changes: I expected to see far fewer changes. Can you review and see whether you can address this? (I expect you might be best of to create a new branch from master, cherry pick the changes you actually want and force push that result to the feature/disable-test-history branch.)

@AlexNetcare AlexNetcare force-pushed the feature/disable-test-history branch from 7150b33 to 9d6aca2 Compare December 17, 2024 15:10
@AlexNetcare
Copy link
Contributor Author

Hello @fhoeben, I did see that my rebase was definitely wrong and I wanted to fix it before writing a comment about it. But I was too slow.
As suggested, I created an extra branch and force-pushed it onto this one. So it should be up to date now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants