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

Delete jdk11_tier1_buffer for duplicate testcases #4018

Closed
wants to merge 1 commit into from

Conversation

sendaoYan
Copy link
Contributor

jdk11_tier1_buffer(sanity.openjdk) is subset of jdk_nio(extened.openjdk), which cause there are 13 duplicate testcases in sanity.openjdk and extended.openjdk. Delete jdk11_tier1_buffer in openjdk/playlist.xml to avoid duplicate testcases

Fixes: #3998

Signed-off-by: sendaoYan yansendao.ysd@alibaba-inc.com

jdk11_tier1_buffer(sanity.openjdk) is subset of jdk_nio(extened.openjdk), which cause there are 13 duplicate testcases in sanity.openjdk and extended.openjdk. Delete jdk11_tier1_buffer in openjdk/playlist.xml to avoid duplicate testcases

Fixes: adoptium#3998
Signed-off-by: sendaoYan <yansendao.ysd@alibaba-inc.com>
@llxia
Copy link
Contributor

llxia commented Oct 12, 2022

I think there may be a reason that we keep jdk11_tier1_buffer in sanity level. Could you just remove $(Q)$(JTREG_JDK_TEST_DIR)/java/nio/Buffer$(Q) from jdk_nio?

@sendaoYan
Copy link
Contributor Author

sendaoYan commented Oct 13, 2022

I think there may be a reason that we keep jdk11_tier1_buffer in sanity level. Could you just remove $(Q)$(JTREG_JDK_TEST_DIR)/java/nio/Buffer$(Q) from jdk_nio?

There is a solution: Get testlist of jdk11_tier1_buffer before start jdk_nio test, then add testlist of jdk11_tier1_buffer as exclude file. This solution will keep jdk11_tier1_buffer in sanity.openjdk, and minus jdk11_tier1_buffer from jdk_nio in extended.openjdk.

diff --git a/openjdk/playlist.xml b/openjdk/playlist.xml
index aac12fe3..d12096b2 100644
--- a/openjdk/playlist.xml
+++ b/openjdk/playlist.xml
@@ -798,6 +798,18 @@
        -exclude:$(Q)$(TEST_RESROOT)$(D)$(PROBLEM_LIST_FILE)$(Q) \
        ${FEATURE_PROBLEM_LIST_FILE} \
        ${VENDOR_PROBLEM_LIST_FILE} \
+       -l $(Q)$(JTREG_JDK_TEST_DIR)/java/nio/Buffer$(Q) \
+       > $(Q)$(REPORTDIR)$(D)jdk11_tier1_buffer-list.txt$(Q); \
+       $(JAVA_COMMAND) -Xmx512m -jar $(Q)$(TEST_RESROOT)$(D)jtreg$(D)lib$(D)jtreg.jar$(Q) \
+       $(JTREG_BASIC_OPTIONS) $(JDK_NATIVE_OPTIONS) -vmoptions:$(Q)-Xmx512m $(JVM_OPTIONS) $(VMOPTION_HEADLESS)$(Q) \
+       -w $(Q)$(REPORTDIR)$(D)work$(Q) \
+       -r $(Q)$(REPORTDIR)$(D)report$(Q) \
+       -jdk:$(Q)$(TEST_JDK_HOME)$(Q) \
+       -exclude:$(Q)$(JTREG_JDK_TEST_DIR)$(D)ProblemList.txt$(Q) \
+       -exclude:$(Q)$(TEST_RESROOT)$(D)$(PROBLEM_LIST_FILE)$(Q) \
+       -exclude:$(Q)$(REPORTDIR)$(D)jdk11_tier1_buffer-list.txt$(Q) \
+       ${FEATURE_PROBLEM_LIST_FILE} \
+       ${VENDOR_PROBLEM_LIST_FILE} \
        $(Q)$(JTREG_JDK_TEST_DIR):jdk_nio$(Q); \
        $(TEST_STATUS)</command>
                <levels>

issue3998.patch.txt

test result log file:

run-2.log

image

result diff:

image

@llxia
Copy link
Contributor

llxia commented Oct 13, 2022

@smlambert @sophia-guo any preference? I think we have jdk11_tier1_buffer at the sanity level because it is part of tier1. By removing this target, we will run jdk11_tier1_buffer at the extended level as part of jdk_nio.

@sophia-guo
Copy link
Contributor

@llxia yeah, same for other duplicate testcases. #4014 (comment)

@smlambert
Copy link
Contributor

I think ultimately our goal should be to match sanity.openjdk to Tier1.

To avoid duplication, can the targets that are currently in extended be broken down so its a smaller set of sub-targets being run in extended. ?

@sophia-guo
Copy link
Contributor

@smlambert that could be a very simple change in upstream openjdk repo. However if this repo the change may not be that easy or straightforward.

@llxia
Copy link
Contributor

llxia commented Oct 13, 2022

@sophia-guo Do you mean to contribute the change to the OpenJDK repo or our mirror repo? If we just make a change in our mirror repo, we need to make sure all vendors' extension repos get updated and we may have a potential risk of a future merge conflict.

@sophia-guo
Copy link
Contributor

I'm referring the upstream openjdk repo, not the mirror ones. Though contribute to upstream openjdk requires permission and it takes a long process. Also upstream may not be interested with the change. I'm just offering all possibilities.

@llxia
Copy link
Contributor

llxia commented Oct 13, 2022

Ideally, we should not have any duplication. However, we would like to keep tier1 at the sanity level as we run them nightly. The extended level often runs weekly due to farm capacity. By only running tests in extended, we reduce the test frequency which may result in defeats being escaped and discovered at a later stage.

In this case, jdk11_tier1_buffer only takes ~28 secs to run. Yes, it is duplication, but it does not take long. If we do not have a good alternative solution, could we just keep it as is?

@smlambert
Copy link
Contributor

smlambert commented Oct 13, 2022

Yes, I think some duplication is reasonable, and when looking at the upstream organization of material (4 tiers) there is duplication there too (see com/sun/crypto/provider/Cipher, java/nio/Buffer and sun/nio/cs/ISO8859x.java as examples). EDIT, no there is not duplication misread the TEST.groups file, com/sun/crypto/provider/Cipher, java/nio/Buffer and sun/nio/cs/ISO8859x.java are with a - in front in tier2.

I wanted a diagram of what the openjdk(upstream) make targets that make up tier1, then I wanted to see what other targets remain. Then I wanted to relate it to our levels (sanity, extended, special, dev).

Currently there are 4 tiers in upstream(openjdk), this shows the contents of them (for jdk11+):

graph TD;

tier1-->tier1_part1
tier1-->tier1_part2 
tier1-->tier1_part3

tier1_part1--level:sanity-->jdk_lang
tier1_part2--level:sanity-->jdk_util
tier1_part3--level:sanity-->jdk_math
tier1_part3--level:sanity-->jdk_svc_sanity
tier1_part3--level:sanity-->java/nio/Buffer
tier1_part3--level:extended in jdk_security2-->com/sun/crypto/provider/Cipher
tier1_part3--level:sanity-->sun/nio/cs/ISO8859x.java 
tier1_part3--level:sanity-->tools/pack200
Loading
graph TD;
tier2-->tier2_part1
tier2-->tier2_part2
tier2-->tier2_part3

tier2_part1-->jdk_security

tier2_part1--level:extended in jdk_security2-->-com/sun/crypto/provider/Cipher
tier2_part2--level:extended-->core_tools
tier2_part2--level:sanity-->-tools/pack200 
tier2_part2--level:extended-->jdk_io
tier2_part2--level:extended-->jdk_nio
tier2_part2--level:sanity-->-java/nio/Buffer
tier2_part2--level:sanity-->-sun/nio/cs/ISO8859x.java 
tier2_part2--level:extended-->jdk_other
tier2_part2--level:extended-->jdk_text
tier2_part2--level:extended-->jdk_time
tier2_part3--level:extended-->jdk_net

jdk_security--level:extended-->jdk_security1
jdk_security--level:extended-->jdk_security2
jdk_security--level:extended-->jdk_security3
jdk_security--level:extended-->jdk_security4

Loading
graph TD;
tier3-->build
tier3-->jdk_rmi  
tier3-->jdk_beans 
tier3-->jdk_imageio 
tier3-->jdk_sound 
tier3-->jdk_client_sanity 
tier3-->jdk_svc 
tier3--everythingNotIn-->-:jdk_svc_sanity 
tier3--everythingNotIn-->-:svc_tools
Loading
graph TD;
tier4--everythingNotIn-->-:tier1
tier4--everythingNotIn-->-:tier2
tier4--everythingNotIn-->-:tier3
tier4--everythingNotIn-->-:tier4

Loading

This should form part of documentation as well.

@sendaoYan
Copy link
Contributor Author

@llxia @sophia-guo @smlambert Does the solution descript in #4018 (comment) is acceptable?

@llxia
Copy link
Contributor

llxia commented Oct 18, 2022

@sendaoYan May I know if there are other reasons for removing the jdk11_tier1_buffer except it is a duplication?

@sendaoYan
Copy link
Contributor Author

ay I know if there are other reasons for removing the jdk11_tier1_buff

No, just for the duplication reason.

@sendaoYan
Copy link
Contributor Author

I think we should close this PR.

@sendaoYan sendaoYan closed this May 13, 2024
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.

jdk11_tier1_buffer is subset of jdk_nio, which cause 13 duplicated testcases
4 participants