RFR: 8258525: Some existing tests should use /nodynamiccopyright/ instead of the standard header [v2]

Guoxiong Li github.com+13688759+lgxbslgx at openjdk.java.net
Tue Dec 29 07:36:32 UTC 2020


> Hi all,
> 
> This is the background of this patch. According to the comments[1] written by Jonathan Gibbons during the review of JDK-8231622[2], I want to solve previous tests that used the bad style. I wrote a python script[3] to find the bad style tests. But it is not precise and complete.
> 
> [1] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015542.html
> [2] https://github.com/openjdk/jdk/pull/1626
> [3] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015619.html
> 
> Currently, I write another python script that can accurately check all the bad style tests. The procedure of this script is shown below.
> 1.  Add a blank line at the head of the java source files(exclude the files contain `/nodynamiccopyright/`) in directory `test/langtools/tools/javac`. This step simulates the changes of the length in the legal header.
> 2. Run the test by using the command `make test CONF=linux-x86_64-server-release JOBS=4 TEST=test/langtools/tools/javac`. This step checks whether the tests can be comfortable to the changes of the length in legal header. If a test failed, it means the test style is bad and need to be revised.
> 3. Delete the blank line at the head of the java source files(exclude the files contain `/nodynamiccopyright/`) in directory `test/langtools/tools/javac`. This step recovers the modified files.
> 
> If all the tests are in good style,  all the tests in step 2 will passed. I use this script to test the branch `master` locally. 129 tests failed. I fixed them. Currently, I use this script to test the branch `test-copyright-clean` of this patch locally. All the tests passed.
> 
> I think this patch solves all the bad style tests if my script has no bugs and no new bad style tests were integrated to branch `master` recently.
> 
> The code of the new python script is shown below. You maybe need to revise the options `CONF=linux-x86_64-server-release` and `JOBS=4` when testing locally.
> 
> #!/usr/bin/python
> import os
> 
> 
> def runTests(find_dir):
>     os.system('make test CONF=linux-x86_64-server-release JOBS=4 TEST=' + find_dir)
>     return
> 
> 
> def addBlankLinePerFile(full_path):
>     search_file = open(full_path, 'r+')
>     content = search_file.read()
>     if content.find("/nodynamiccopyright/") < 0 and full_path.endswith(".java"):
>         search_file.seek(0)
>         search_file.write("\n")
>         search_file.write(content)
>     search_file.close()
> 
> 
> def deleteBlankLinePerFile(full_path):
>     search_file = open(full_path, 'r')
>     old_content = search_file.readlines()
>     search_file.close()
>     search_file = open(full_path, 'w')
>     if ''.join(old_content).find("/nodynamiccopyright/") < 0 and full_path.endswith(".java"):
>         new_content = ''.join(old_content[1:])
>         search_file.write(new_content)
>     else:
>         search_file.write(''.join(old_content))
>     search_file.close()
> 
> 
> def addBlankLine(find_dir):
>     for dir_path, dir_names, file_names in os.walk(find_dir):
>         for file_name in file_names:
>             full_path = os.path.join(dir_path, file_name)
>             addBlankLinePerFile(full_path)
> 
> 
> def deleteBlankLine(find_dir):
>     for dir_path, dir_names, file_names in os.walk(find_dir):
>         for file_name in file_names:
>             full_path = os.path.join(dir_path, file_name)
>             deleteBlankLinePerFile(full_path)
> 
> 
> if __name__ == "__main__":
>     findDir = "test/langtools/tools/javac"
>     addBlankLine(findDir)
>     runTests(findDir)
>     deleteBlankLine(findDir)
> 
> 
> Thank you for taking the time to review.
> And could I ask your help to create a new issue in the bug tracker? Thanks again.

Guoxiong Li has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:

 - Merge branch 'master' into test-copyright-cleanup
 - Merge branch 'master' into test-copyright-cleanup
 - Merge branch 'master' into test-copyright-cleanup
 - Some tests should use /nodynamiccopyright/ instead of the copyright header.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1732/files
  - new: https://git.openjdk.java.net/jdk/pull/1732/files/67882939..eeb04044

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1732&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1732&range=00-01

  Stats: 36027 lines in 1037 files changed: 27372 ins; 5145 del; 3510 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1732.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1732/head:pull/1732

PR: https://git.openjdk.java.net/jdk/pull/1732


More information about the compiler-dev mailing list