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

Remove unneeded injected test #393

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

timja
Copy link
Member

@timja timja commented Feb 16, 2022

As of jenkinsci/jenkins#5923 f:radio uses for= to attach labels by generating a unique ID per input.

Theme manager is using a similar (completely safe) method.

Can we remove this test please given core is using this pattern?

One of the annoying parts of InjectedTest is you can't selectively disable it -.-

@jglick
Copy link
Member

jglick commented Feb 16, 2022

I know nothing about this assertion or the change that allegedly made it incorrect.

@timja timja requested review from uhafner and removed request for jglick February 16, 2022 22:11
@timja
Copy link
Member Author

timja commented Feb 18, 2022

@janfaracik can you approve

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

I think it is fine to remove this test when it does not make sense in all cases.

@timja timja merged commit 5257fc5 into jenkinsci:master Feb 18, 2022
@timja timja deleted the remove-unneeded-injected-test branch February 18, 2022 22:01
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