Skip to content

adding crlf characters for the failing tests in api.templates - #248

Closed
sarveshkesharwani wants to merge 1 commit into
apache:masterfrom
sarveshkesharwani:failing-tests-in-api.templates
Closed

adding crlf characters for the failing tests in api.templates#248
sarveshkesharwani wants to merge 1 commit into
apache:masterfrom
sarveshkesharwani:failing-tests-in-api.templates

Conversation

@sarveshkesharwani

Copy link
Copy Markdown
Contributor

This fixes the failing tests in "SCFTHandlerTest.java" in api.templates module.

@junichi11

Copy link
Copy Markdown
Member

Probably, it's incomplete. I guess that your OS is Windows. Tests would fail in Linux if the changes are added.

BTW, we should remove the license header of utf8.xml [1] because we get an error. (It should be excluded from rat)

Testcase: testUTF8(org.netbeans.modules.templates.SCFTHandlerTest):	Caused an ERROR
class.txt:1:0 Expected an operand but found <
<!--
^ in class.txt at line number 1 at column number 0 Log:

[1] https://github.com/apache/incubator-netbeans/blob/master/api.templates/test/unit/src/org/netbeans/modules/templates/utf8.xml

@JaroslavTulach @jlahoda
Should we consider Windows in tests?

Thanks.

@sarveshkesharwani

Copy link
Copy Markdown
Contributor Author

Yes, my OS is Windows. Were the tests in SCFTHandlerTest.java passing before in linux? (without these crlf changes)

@jlahoda

jlahoda commented Nov 7, 2017

Copy link
Copy Markdown
Contributor

My personal opinion: ideally tests should pass/work on both Linux and Windows. Instead of playing with different line endings in the expected text, I'd suggest to normalize to one line endings format (ideally the Linux one). I.e. the expected text would have only one format ('\n') of line endings, just readFile(newForm) would do '.replace("\r\n", "\n")' or '.replace(System.getProperty("line.separator"), "\n")'.

@junichi11

Copy link
Copy Markdown
Member

Were the tests in SCFTHandlerTest.java passing before in linux? (without these crlf changes)

Yes, other than the above problem I wrote. (I created a new PR #255)

@jlahoda Thank you for your help! Great idea.

@sarveshkesharwani Could you please fix it? (Please see Jan's comment)

@sarveshkesharwani

Copy link
Copy Markdown
Contributor Author

@jlahoda Thank you. That's really a great idea.
@junichi11 Fixed. Thanks a lot for the review.

@JaroslavTulach

JaroslavTulach commented Nov 8, 2017

Copy link
Copy Markdown

There are two jobs running (some of) our tests: windows and linux. I believe it is important to stabilize them both - e.g. it is desirable that our tests pass on Windows as well as Linux.

Btw. it is unfortunate the windows job has been broken for a while and nobody paid attention to that (reported as #127). Ideas how to make stable test runs priority for the community are welcomed.

@ebarboni

ebarboni commented Nov 8, 2017

Copy link
Copy Markdown
Contributor

@JaroslavTulach maybe we should connect jenkins to dev mailing list ?
see [1] section how do I allow Jenkins to mail to my project's "dev" list?

[1] https://wiki.apache.org/general/Jenkins?action=show&redirect=Hudson#Are_there_recommended_or_mandatory_Jenkins_settings_for_ASF_projects.3F

@junichi11

Copy link
Copy Markdown
Member

@sarveshkesharwani Great. Thanks a lot! It would be nice if you can rebase and squash your changes into one commit with a proper commit message.

@JaroslavTulach That makes sense. Thank you for your help!

…rTest - windows (replace crlf characters with lf)
@sarveshkesharwani
sarveshkesharwani force-pushed the failing-tests-in-api.templates branch from ae8c0a6 to 7189070 Compare November 9, 2017 10:14
@sarveshkesharwani

Copy link
Copy Markdown
Contributor Author

@junichi11 That makes sense. Done.

@junichi11 junichi11 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perfect. Thank you!

@junichi11

Copy link
Copy Markdown
Member

@sarveshkesharwani BTW, Did you send iCLA[1]? (I'm not sure whether this change requires it.) If no and you are going to continue contributing to Apache NetBeans, you should send it :)

[1] https://www.apache.org/licenses/#clas

@sarveshkesharwani

Copy link
Copy Markdown
Contributor Author

@junichi11 I haven't sent an iCLA yet, but I am definitely going to continue contributing to Apache NetBeans. I will send the iCLA once I have made a little more contribution towards Apache NetBeans. :)

@junichi11

Copy link
Copy Markdown
Member

@sarveshkesharwani Great to hear. We can merge your change without problems after you send it.

@asfgit asfgit closed this in af3dbdb Nov 25, 2017
@matthiasblaesing

Copy link
Copy Markdown
Contributor

Merged as "trivial fix" without an iCLA. Thanks for looking into this.

@sarveshkesharwani
sarveshkesharwani deleted the failing-tests-in-api.templates branch August 6, 2018 05:08
Achal1607 pushed a commit to Achal1607/netbeans that referenced this pull request Feb 21, 2025
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.

6 participants