You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/01/13 14:30:36 UTC
[GitHub] [solr] cpoerschke commented on a change in pull request #497: SOLR-15852 Update dev-tools/scripts for 9.0
cpoerschke commented on a change in pull request #497:
URL: https://github.com/apache/solr/pull/497#discussion_r783956542
##########
File path: dev-tools/scripts/buildAndPushRelease.py
##########
@@ -182,48 +202,40 @@ def normalizeVersion(tup):
tup = tup + ('0',)
return '.'.join(tup) + suffix
-def pushLocal(version, root, rev, rcNum, localDir):
+
+def pushLocal(version, root, rcNum, localDir):
print('Push local [%s]...' % localDir)
os.makedirs(localDir)
- dir = 'lucene-solr-%s-RC%d-rev%s' % (version, rcNum, rev)
- os.makedirs('%s/%s/lucene' % (localDir, dir))
- os.makedirs('%s/%s/solr' % (localDir, dir))
- print(' Lucene')
- lucene_dist_dir = '%s/lucene/packaging/build/distributions' % root
- os.chdir(lucene_dist_dir)
- print(' zip...')
- if os.path.exists('lucene.tar.bz2'):
- os.remove('lucene.tar.bz2')
- run('tar cjf lucene.tar.bz2 *')
-
- os.chdir('%s/%s/lucene' % (localDir, dir))
- print(' unzip...')
- run('tar xjf "%s/lucene.tar.bz2"' % lucene_dist_dir)
- os.remove('%s/lucene.tar.bz2' % lucene_dist_dir)
+ rev = open('%s/solr/distribution/build/release/.gitrev' % root, encoding='UTF-8').read()
+ dir = 'solr-%s-RC%d-rev-%s' % (version, rcNum, rev)
Review comment:
noting `rev%s` to `rev-%s` change here.
##########
File path: dev-tools/scripts/releaseWizard.py
##########
@@ -1640,15 +1653,15 @@ def run(self):
for line in cmd.display_cmd():
print(" %s" % line)
print()
+ confirm_each = (not self.confirm_each_command is False) and len(commands) > 1
if not self.enable_execute is False:
if self.run_text:
print("\n%s\n" % self.get_run_text())
- if len(commands) > 1:
- if not self.confirm_each_command is False:
- print("You will get prompted before running each individual command.")
- else:
- print(
- "You will not be prompted for each command but will see the ouput of each. If one command fails the execution will stop.")
+ if confirm_each:
+ print("You will get prompted before running each individual command.")
+ else:
+ print(
+ "You will not be prompted for each command but will see the ouput of each. If one command fails the execution will stop.")
Review comment:
```suggestion
"You will not be prompted for each command but will see the output of each. If one command fails the execution will stop.")
```
##########
File path: dev-tools/scripts/smokeTestRelease.py
##########
@@ -169,119 +160,92 @@ def checkJARMetaData(desc, jarFile, gitRevision, version):
if gitRevision != 'skip':
# Make sure this matches the version and git revision we think we are releasing:
- # TODO: LUCENE-7023: is it OK that Implementation-Version's value now spans two lines?
- verifyRevision = 'Implementation-Version: %s %s' % (version, gitRevision)
- if s.find(verifyRevision) == -1:
- raise RuntimeError('%s is missing "%s" inside its META-INF/MANIFEST.MF (wrong git revision?)' % \
+ match = re.search("Implementation-Version: (.+\r\n .+)", s, re.MULTILINE)
+ if match:
+ implLine = match.group(1).replace("\r\n ", "")
+ verifyRevision = '%s %s' % (version, gitRevision)
+ if implLine.find(verifyRevision) == -1:
+ raise RuntimeError('%s is missing "%s" inside its META-INF/MANIFEST.MF (wrong git revision?)' % \
(desc, verifyRevision))
+ else:
+ raise RuntimeError('%s is missing Implementation-Version inside its META-INF/MANIFEST.MF' % desc)
notice = decodeUTF8(z.read(NOTICE_FILE_NAME))
license = decodeUTF8(z.read(LICENSE_FILE_NAME))
- justFileName = os.path.split(desc)[1]
-
- if justFileName.lower().find('solr') != -1:
- if SOLR_LICENSE is None:
- raise RuntimeError('BUG in smokeTestRelease!')
- if SOLR_NOTICE is None:
- raise RuntimeError('BUG in smokeTestRelease!')
- if notice != SOLR_NOTICE:
- raise RuntimeError('%s: %s contents doesn\'t match main NOTICE.txt' % \
- (desc, NOTICE_FILE_NAME))
- if license != SOLR_LICENSE:
- raise RuntimeError('%s: %s contents doesn\'t match main LICENSE.txt' % \
- (desc, LICENSE_FILE_NAME))
- else:
- if LUCENE_LICENSE is None:
- raise RuntimeError('BUG in smokeTestRelease!')
- if LUCENE_NOTICE is None:
- raise RuntimeError('BUG in smokeTestRelease!')
- if notice != LUCENE_NOTICE:
- raise RuntimeError('%s: %s contents doesn\'t match main NOTICE.txt' % \
- (desc, NOTICE_FILE_NAME))
- if license != LUCENE_LICENSE:
- raise RuntimeError('%s: %s contents doesn\'t match main LICENSE.txt' % \
- (desc, LICENSE_FILE_NAME))
+ if SOLR_LICENSE is None:
+ raise RuntimeError('BUG in smokeTestRelease!')
+ if SOLR_NOTICE is None:
+ raise RuntimeError('BUG in smokeTestRelease!')
Review comment:
minor: could combine
```suggestion
if SOLR_LICENSE is None or SOLR_NOTICE is None:
raise RuntimeError('BUG in smokeTestRelease!')
```
##########
File path: dev-tools/scripts/smokeTestRelease.py
##########
@@ -537,106 +502,100 @@ def getDirEntries(urlString):
return l
else:
links = getHREFs(urlString)
- for i, (text, subURL) in enumerate(links):
+ for i, (text, subURL) in enumerate(links): # pylint: disable=unused-variable
if text == 'Parent Directory' or text == '..':
return links[(i+1):]
+ return None
+
-def unpackAndVerify(java, project, tmpDir, artifact, gitRevision, version, testArgs, baseURL):
+def unpackAndVerify(java, tmpDir, artifact, gitRevision, version, testArgs):
destDir = '%s/unpack' % tmpDir
if os.path.exists(destDir):
shutil.rmtree(destDir)
os.makedirs(destDir)
os.chdir(destDir)
print(' unpack %s...' % artifact)
- unpackLogFile = '%s/%s-unpack-%s.log' % (tmpDir, project, artifact)
+ unpackLogFile = '%s/solr-unpack-%s.log' % (tmpDir, artifact)
if artifact.endswith('.tar.gz') or artifact.endswith('.tgz'):
run('tar xzf %s/%s' % (tmpDir, artifact), unpackLogFile)
elif artifact.endswith('.zip'):
run('unzip %s/%s' % (tmpDir, artifact), unpackLogFile)
# make sure it unpacks to proper subdir
l = os.listdir(destDir)
- expected = '%s-%s' % (project, version)
+ expected = 'solr-%s' % version
if l != [expected]:
raise RuntimeError('unpack produced entries %s; expected only %s' % (l, expected))
unpackPath = '%s/%s' % (destDir, expected)
- verifyUnpacked(java, project, artifact, unpackPath, gitRevision, version, testArgs, tmpDir, baseURL)
+ verifyUnpacked(java, artifact, unpackPath, gitRevision, version, testArgs)
return unpackPath
-LUCENE_NOTICE = None
-LUCENE_LICENSE = None
SOLR_NOTICE = None
SOLR_LICENSE = None
-def verifyUnpacked(java, project, artifact, unpackPath, gitRevision, version, testArgs, tmpDir, baseURL):
- global LUCENE_NOTICE
- global LUCENE_LICENSE
+def is_in_list(in_folder, files, indent=4):
+ for fileName in files:
+ print("%sChecking %s" % (" "*indent, fileName))
+ found = False
+ for f in [fileName, fileName + '.txt', fileName + '.md']:
+ if f in in_folder:
+ in_folder.remove(f)
+ found = True
+ if not found:
+ raise RuntimeError('file "%s" is missing' % fileName)
Review comment:
`file_name` rather than `fileName` is preferred in python as far as i know and since this function is new maybe nice to follow the python convention for it, noting that the `in_folder` argument already does.
```suggestion
for file_name in files:
print("%sChecking %s" % (" "*indent, file_name))
found = False
for f in [file_name, file_name + '.txt', file_name + '.md']:
if f in in_folder:
in_folder.remove(f)
found = True
if not found:
raise RuntimeError('file "%s" is missing' % file_name)
```
##########
File path: dev-tools/scripts/smokeTestRelease.py
##########
@@ -861,88 +756,40 @@ def testSolrExample(unpackPath, javaPath, isSrc):
else:
os.chdir(unpackPath)
-# check for broken links
-def checkBrokenLinks(path):
- # also validate html/check for broken links
- if checkJavadocLinks.checkAll(path):
- raise RuntimeError('broken javadocs links found!')
-
-def testDemo(run_java, isSrc, version, jdk):
Review comment:
ah! here's the `testDemo` method being removed which the `pylint` was flagging up as undefined at line 630 above.
##########
File path: dev-tools/scripts/smokeTestRelease.py
##########
@@ -1264,155 +1068,48 @@ def parse_config():
reVersion1 = re.compile(r'\>(\d+)\.(\d+)\.(\d+)(-alpha|-beta)?/\<', re.IGNORECASE)
reVersion2 = re.compile(r'-(\d+)\.(\d+)\.(\d+)(-alpha|-beta)?\.', re.IGNORECASE)
-def getAllLuceneReleases():
- s = load('https://archive.apache.org/dist/lucene/java')
-
- releases = set()
- for r in reVersion1, reVersion2:
- for tup in r.findall(s):
- if tup[-1].lower() == '-alpha':
- tup = tup[:3] + ('0',)
- elif tup[-1].lower() == '-beta':
- tup = tup[:3] + ('1',)
- elif tup[-1] == '':
- tup = tup[:3]
- else:
- raise RuntimeError('failed to parse version: %s' % tup[-1])
- releases.add(tuple(int(x) for x in tup))
-
- l = list(releases)
- l.sort()
- return l
-
-def confirmAllReleasesAreTestedForBackCompat(smokeVersion, unpackPath):
-
- print(' find all past Lucene releases...')
- allReleases = getAllLuceneReleases()
- #for tup in allReleases:
- # print(' %s' % '.'.join(str(x) for x in tup))
-
- testedIndices = set()
-
- os.chdir(unpackPath)
-
- print(' run TestBackwardsCompatibility..')
- command = 'ant test -Dtestcase=TestBackwardsCompatibility -Dtests.verbose=true'
- p = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- stdout, stderr = p.communicate()
- if p.returncode != 0:
- # Not good: the test failed!
- raise RuntimeError('%s failed:\n%s' % (command, stdout))
- stdout = stdout.decode('utf-8',errors='replace').replace('\r\n','\n')
-
- if stderr is not None:
- # Should not happen since we redirected stderr to stdout:
- raise RuntimeError('stderr non-empty')
-
- reIndexName = re.compile(r'TEST: index[\s*=\s*](.*?)(-cfs|-nocfs)$', re.MULTILINE)
- for name, cfsPart in reIndexName.findall(stdout):
- # Fragile: decode the inconsistent naming schemes we've used in TestBWC's indices:
- #print('parse name %s' % name)
- tup = tuple(name.split('.'))
- if len(tup) == 3:
- # ok
- tup = tuple(int(x) for x in tup)
- elif tup == ('4', '0', '0', '1'):
- # CONFUSING: this is the 4.0.0-alpha index??
- tup = 4, 0, 0, 0
- elif tup == ('4', '0', '0', '2'):
- # CONFUSING: this is the 4.0.0-beta index??
- tup = 4, 0, 0, 1
- elif name == '5x-with-4x-segments':
- # Mixed version test case; ignore it for our purposes because we only
- # tally up the "tests single Lucene version" indices
- continue
- elif name == '5.0.0.singlesegment':
- tup = 5, 0, 0
- else:
- raise RuntimeError('could not parse version %s' % name)
-
- testedIndices.add(tup)
-
- l = list(testedIndices)
- l.sort()
- if False:
- for release in l:
- print(' %s' % '.'.join(str(x) for x in release))
-
- allReleases = set(allReleases)
-
- for x in testedIndices:
- if x not in allReleases:
- # Curious: we test 1.9.0 index but it's not in the releases (I think it was pulled because of nasty bug?)
- if x != (1, 9, 0):
- raise RuntimeError('tested version=%s but it was not released?' % '.'.join(str(y) for y in x))
-
- notTested = []
- for x in allReleases:
- if x not in testedIndices:
- releaseVersion = '.'.join(str(y) for y in x)
- if releaseVersion in ('1.4.3', '1.9.1', '2.3.1', '2.3.2'):
- # Exempt the dark ages indices
- continue
- if x >= tuple(int(y) for y in smokeVersion.split('.')):
- # Exempt versions not less than the one being smoke tested
- print(' Backcompat testing not required for release %s because it\'s not less than %s'
- % (releaseVersion, smokeVersion))
- continue
- notTested.append(x)
-
- if len(notTested) > 0:
- notTested.sort()
- print('Releases that don\'t seem to be tested:')
- failed = True
- for x in notTested:
- print(' %s' % '.'.join(str(y) for y in x))
- raise RuntimeError('some releases are not tested by TestBackwardsCompatibility?')
- else:
- print(' success!')
-
def main():
c = parse_config()
- scriptVersion = scriptutil.find_current_version()
+ # Pick <major>.<minor> part of version and require script to be from same branch
+ scriptVersion = re.search(r'((\d+).(\d+)).(\d+)', scriptutil.find_current_version()).group(1).strip()
if not c.version.startswith(scriptVersion + '.'):
raise RuntimeError('smokeTestRelease.py for %s.X is incompatible with a %s release.' % (scriptVersion, c.version))
print('NOTE: output encoding is %s' % sys.stdout.encoding)
smokeTest(c.java, c.url, c.revision, c.version, c.tmp_dir, c.is_signed, c.local_keys, ' '.join(c.test_args),
downloadOnly=c.download_only)
+
def smokeTest(java, baseURL, gitRevision, version, tmpDir, isSigned, local_keys, testArgs, downloadOnly=False):
startTime = datetime.datetime.now()
- # disable flakey tests for smoke-tester runs:
- testArgs = '-Dtests.badapples=false %s' % testArgs
-
+ # Tests annotated @Nightly are more resource-intensive but often cover
+ # important code paths. They're disabled by default to preserve a good
+ # developer experience, but we enable them for smoke tests where we want good
+ # coverage. Still we disable @BadApple tests
+ testArgs = '-Dtests.nigthly=true -Dtests.badapples=false %s' % testArgs
Review comment:
```suggestion
testArgs = '-Dtests.nightly=true -Dtests.badapples=false %s' % testArgs
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org