-
Notifications
You must be signed in to change notification settings - Fork 24
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
Initial section on testing #30
Conversation
👋 Welcome back jwilhelm! A progress list of the required criteria for merging this PR into |
Webrevs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I understand the desire to have as much relative information in the guide, I don't think we need to repeat ourselves, most of the information presented here is already available in doc/testing.md
and doc/hotspot-unit-tests.md
. the former document provides more comprehensive documentation about all available/supported testing in OpenJDK, different modes of test execution; the latter gives an overview of different TEST*
macros available in hotspot gtest, defines good tests, provides guidelines on writing such tests, etc.
I think we need to find some middle ground here, so we won't duplicate documentation and at the same time, we won't force people to gather required information from N different places. With that in mind, I think it's important to 1st define the goal of 'testing' section of the Guide.
Mailing list message from Joe Darcy on guide-dev: Hi Jesper, Besides the logistical information, I think it would be helpful to add For example, by default changes should include regression tests, unless Thanks, -Joe On 10/16/2020 9:04 AM, Jesper Wilhelmsson wrote: |
The documents in /doc/ requires prior knowledge to understand. Those aren't written for beginners. The goal for the guide in this case is to provide quick links to more information and serve as a buffer to fill in the gaps for a first time tester. I agree that we should avoid duplication. For the Developers' Guide I want to have just enough information to get everything up and running. For any additional information there should be links to more detailed documentation. Personally I have issues with documentation that requires knowledge not only to understand, but also to access. But as the source is now on GitHub and the .md files are reasonably readable through the web interface it's at least possible to link to them. I don't know if search engines will index the JDK source code though so for people googling for documentation we still need some indexed document that refers to these documents. |
Yes, I think this is a good idea. There is some of this in /doc/hotspot-unit-tests.md but it's GTest specific and very HotSpot centric, so we should have something that is more generic and suitable for all. I have updated with a few sentences about this. Let me know if you think this needs to be expanded more or have any other suggestions. |
Writing jtreg tests provides generic guidelines about good jtreg tests, but I'd say it's generic enough (as it should be as it's part of JTReg's documentation, not OpenJDK documentation) and doesn't talk about things which are specific to OpenJDK, e.g. keywords, best practices, common libraries, etc. langtools guidelines, on the other hand, provides langtools specific guidelines, some of them are adopted by other test suites, some not. I think it makes sense for us to compose some OpenJDK-centric but generic enough guidelines and put them into either this guide or into |
Mailing list message from Igor Ignatyev on guide-dev:
what I meant by b) is to have C/C++ test code which calls functions exported by JDK/JVM (such as `JNI_CreateJavaVM`, `ZIP_Open`, `JAWT_DrawingSurfaceInfo::GetDrawingSurfaceInfo`, `jvmtiEnv::GetAllModules`, etc), have that code compiled into shared library and/or executable; and then use in a jtreg test which has `/native`. A simplest example would be `test/hotspot/jtreg/native_sanity/JniVersion.java` and `libJniVersion.c`.
I'm totally fine w/ being an uncredited contributor, it's not about fame, it's about doing the right :) |
Ok, I get the high level of what you say here, but I don't have enough detail about how this works to be able to write a comprehensive description about it.
Right. At some point I would like this project to have a few more Committers/Reviewers though, so it for purely selfish reasons that I want you to be famous ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any particular comments on text itself, but IMO we shouldn't use a third-party markdown rendering service for links in OpenJDK documentation. I suggest to either link directly to HTML content hosted on https://openjdk.java.net or link to the markdown source files in the jdk repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the addition of some conceptual testing information is sufficient for now, with the possibility of adding more discussion later.
Mailing list message from Lance Andersen on guide-dev: Understand, and I guess my point was should the use of TEST.properties be covered in the dev guide of just left to the JTREG pages. It would be nice to have best practices flushed out. Best Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 |
Hi Lance, I agree that we need to write down the best practices and have them easily available and discoverable, but I don't think that the Guide is the right place for them. I see the Guide as a QuickStart or How To Start Hacking OpenJDK In 5 Minutes (as opposed to The Comprehension OpenJDK Guide: How To Do All Possible Things Right And What Things Not To Do). if we start to add all best practices to the guide, inevitably they will either be superficially covered or we end up with lots of information and overwhelm readers, both cases will result in poor comprehension and I'd argue would undermine the purpose of this document and will cause more confusion. so I'd prefer not to cover Cheers, |
Hi Igor,
On Oct 22, 2020, at 12:31 PM, Igor Ignatev ***@***.***> wrote:
Mailing list message from Lance Andersen ***@***.***> on guide-dev ***@***.***>:
Understand, and I guess my point was should the use of TEST.properties be covered in the dev guide of just left to the JTREG pages. It would be nice to have best practices flushed out.
Best
Lance
Hi Lance,
I agree that we need to write down the best practices and have them easily available and discoverable, but I don't think that the Guide is the right place for them. I see the Guide as a QuickStart or How To Start Hacking OpenJDK In 5 Minutes (as opposed to The Comprehension OpenJDK Guide: How To Do All Possible Things Right And What Things Not To Do). if we start to add all best practices to the guide, inevitably they will either be superficially covered or we end up with lots of information and overwhelm readers, both cases will result in poor comprehension and I'd argue would undermine the purpose of this document and will cause more confusion.
so I'd prefer not to cover TEST.properties here, instead, I suggest creating a ticket to document common best practices in /doc/jtreg-tests.md in the jdk repo, and list TEST.properties as one of the things we need to document.
Having an additional best practices document would be fine also and perhaps the dev guide just points to it. I do think we need this as this will help with bringing new developers up to speed quicker as we all have a lot of information that we have learned over the years but it is not written down :-)
Best
Lance
Cheers,
-- Igor
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQPFO2XCJGC6AZF7SRDK2TSMBM5PANCNFSM4STQRI7A>.
Best
Lance
------------------
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen@oracle.com
|
The Guide is intended to cover best practices so I think this is the best place for such documentation. In my opinion only docs that are tied to a specific version of the source code should be in the /doc/ directory. That is not the place for generic guides and documentation. Erik's comments above shows the problem with accessing that documentation - how do we link to it and how do we make it readable without requiring people to download and build the JDK? People expect to find documentation on-line these days. Wether it's overwhelming or comprehensible is a matter of layout. Even large amounts of information can be made easily accessible with the right layout. I have planned to add a "Quick links"-box at the top of each section, with the relevant links for expert users who just want to get to the technical documentation. In a similar fashion we can have a "Pro-tip" box with some of the more advanced guidelines that might not be relevant for the beginner. It's not a problem to have a lot of text in the Guide as long as it's easily navigateable - and I'm pretty sure most people these days navigate using the search function in their browser. The JTReg documentation is documentation for the tool and should not contain OpenJDK specific guidelines. Adding another document seems to me to be a step in the wrong direction. We already have too many different documents and nobody can find any information. We need to consolidate and reduce the number of places, not add more. |
Hi Jesper,
I am of your mindset on this, but will defer to the groups decision.
On Oct 22, 2020, at 3:31 PM, Jesper Wilhelmsson ***@***.***> wrote:
Having an additional best practices document would be fine also and perhaps the dev guide just points to it.
The Guide is intended to cover best practices so I think this is the best place for such documentation. In my opinion only docs that are tied to a specific version of the source code should be in the /doc/ directory. That is not the place for generic guides and documentation. Erik's comments above shows the problem with accessing that documentation - how do we link to it and how do we make it readable without requiring people to download and build the JDK? People expect to find documentation on-line these days.
Wether it's overwhelming or comprehensible is a matter of layout. Even large amounts of information can be made easily accessible with the right layout. I have planned to add a "Quick links"-box at the top of each section, with the relevant links for expert users who just want to get to the technical documentation. In a similar fashion we can have a "Pro-tip" box with some of the more advanced guidelines that might not be relevant for the beginner. It's not a problem to have a lot of text in the Guide as long as it's easily navigateable - and I'm pretty sure most people these days navigate using the search function in their browser.
The JTReg documentation is documentation for the tool and should not contain OpenJDK specific guidelines. Adding another document seems to me to be a step in the wrong direction. We already have too many different documents and nobody can find any information. We need to consolidate and reduce the number of places, not add more.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQPFOZTPK3PYKXIIAUN4ODSMCCC7ANCNFSM4STQRI7A>.
My best,
Lance
--
Lance Andersen
USTA EMA President and CEO
PTR Professional 5A
USPTA Elite Professional
TIA Global Cardio Tennis-Master Trainer
USRSA
Mobile: 978 857-0446
luckydogtennis.com -- luckydogtennis.com/TennisBlog -- cardiotennis.com
|
Works for me, thank you Jesper!
On Oct 22, 2020, at 4:04 PM, Jesper Wilhelmsson ***@***.***> wrote:
@JesperIRL commented on this pull request.
In src/index.md <#30 (comment)>:
> +
+Below is a small example of a JTReg test. It’s a clean Java class with a main method that is called from the test harness. If the test fails we throw a RuntimeException. This is picked up by the harness and is reported as a test failure. Try to always write a meaningful message in the exception. One that actually helps with understanding what went wrong once the test fails.
+
+ /*
+ * @test
+ * @summary Make sure feature X handles Y correctly
+ * @run main TestXY
+ */
+ public class TestXY {
+ public static void main(String[] args) throws Exception {
+ var result = X.y();
+ if (result != expected_result) {
+ throw new RuntimeException("X.y() gave " + result + ", expexted " + expected_result);
+ }
+ }
+ }
Let's agree on not adding anything about TEST.properties at this point. I have added a note in the todo-list about the need for documented best practices for this. Once the text is written we can discuss where to put it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQPFO7VY2OXME3SM32KAIDSMCF6RANCNFSM4STQRI7A>.
My best,
Lance
--
Lance Andersen
USTA EMA President and CEO
PTR Professional 5A
USPTA Elite Professional
TIA Global Cardio Tennis-Master Trainer
USRSA
Mobile: 978 857-0446
luckydogtennis.com -- luckydogtennis.com/TennisBlog -- cardiotennis.com
|
Unfortunately, any useful documentation will become tied to a specific version, and the Guide already is: you refer to a specific directory layout, the layouts are different in different versions of JDK, tip vs 8u vs 11u; the list of supported test harnesses is version-dependent; even recommendations and some of the best practices are version-dependent; as the presence and content of Actually, I have a slightly different opinion on the whole hypothesis where people expect to find documentation these days. Although it's true for in general people do that online, the information in this Guide and the other related documentation isn't generic information, it's the information for developers. From my experience, most developers expect to find all the needed documentation within their local workspace, not on some external sources. This isn't to say that hosting the same (or similar) documentation on web-servers is useless, it's just not the 1st place I and, I believe, many others would start their search.
that, I totally agree with, but my gut feeling is what the root cause of this problem is the absence of a well-known entry point and poor structure of the documentation. the guide might be such an entry point, or it can be -- Igor |
I assume that you refer to "Code Owners" here. Actually that list is the union of all directories from JDK 9 and forward. Obviously I might have missed some directory that existed for some time in between, but that should be considered a bug. The intention is to cover all versions. Due to the large difference I didn't include JDK 8 and earlier. Maybe that's a bug as well, but my guess is that anyone working on JDK 8 these days already know who owns the code.
If there are significant differences that is relevant to a large part of the community I think we should mention these differences. The Guide isn't only for developers of mainline. Please also note that the fact that these differences aren't mentioned today isn't because the Guide is version specific but rather because we are currently writing it and large parts of it is still missing.
Having the Guide in
Ok. I have a different experience and can only speak for myself. In other open source projects I've looked at I've always used regular web search to find documentation, and so far I haven't seen any project where that documentation wasn't available in a nicely formatted on-line document. Maybe that says more about what kinds of projects I've been looking at rather than where projects in general keep their source code. |
it would appear that when I was talking about version-dependent text, I was mostly talking about this particular section( -- Igor |
@JesperIRL This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 3 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
/integrate |
@JesperIRL Since your change was applied there have been 3 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit 7030605. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
It's a start.
Progress
Reviewers
Download
$ git fetch https://git.openjdk.java.net/guide pull/30/head:pull/30
$ git checkout pull/30