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

Add Phobos public tests extractor script #203

Merged
merged 3 commits into from
Dec 15, 2016

Conversation

wilzbach
Copy link
Member

Adds a simple extraction utility that dumps all public unittests from Phobos in separate files, s.t. we can run them with rdmd and verify that all snippets on dlang.org are runnable (e.g. don't miss imports).

Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

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

Alignment pliz

import std.range;
import std.exception;
import std.algorithm;
import std.path;
Copy link
Member

Choose a reason for hiding this comment

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

On thing I'd like to add to our usual style guidelines is imports should be ordered alphabetically (it did happen at least to me to import something twice when the imports list is long). Also, I wonder how our opinionated community would feel about preferring comma-separated imports instead of one per line.

Copy link
Member Author

@wilzbach wilzbach Dec 10, 2016

Choose a reason for hiding this comment

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

On thing I'd like to add to our usual style guidelines is imports should be ordered alphabetically

Good idea -> dlang-community/D-Scanner#382

Also, I wonder how our opinionated community would feel about preferring comma-separated imports instead of one per line.

I personally prefer line-separated imports because it's easy to add/remove them and the git log is clean.
For such scripts one wants to do sth. like import std.* - I have seen quite many people having their convenience scripting package that public imports most of Phobos


alias visit = ASTVisitor.visit;

override void visit(const Unittest u)
Copy link
Member

Choose a reason for hiding this comment

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

alignment is off

// scan the previous line for ddoc header
auto loc = u.location + 0;
while (sourceCode[loc] != '\n')
loc--;
Copy link
Member

Choose a reason for hiding this comment

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

fix alingment (here, there, and everywhere)


// write the origin source code line
outFile.write("// Line ");
outFile.write(sourceCode[0..loc].count("\n") + 2);
Copy link
Member

Choose a reason for hiding this comment

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

space around operators

"inputdir|i", &inputDir,
"outputdir|o", &outputDir,
"ignore", &ignoredFilesStr,
);
Copy link
Member

Choose a reason for hiding this comment

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

add help strings?

@wilzbach wilzbach force-pushed the add-check-for-public-unittests branch from b9c0147 to 8ff7fe9 Compare December 10, 2016 19:35
@wilzbach
Copy link
Member Author

Alignment pliz

Sorry - parts of this file were a bit older -> dfmted :)


// file name without its parent directory, e.g. std/uni.d
string fileNameNormalized = (inputDir != ".") ? fileName.replace(inputDir,
"")[1 .. $] : fileName[1 .. $];
Copy link
Member

Choose a reason for hiding this comment

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

The line break is surprising. Also I'd factor it like this:

string fileNameNormalized = (inputDir == "." ? fileName : fileName.replace(inputDir, ""))[1 .. $];

writefln("\textractVersion = x%04x", de.extractVersion);
writefln("\tflags = x%04x", de.flags);
writefln("\tcompressionMethod = %d", de.compressionMethod);
writefln("\tcrc32 = x%08x", de.crc32);
writefln("\texpandedSize = %s", de.expandedSize);
writefln("\tcompressedSize = %s", de.compressedSize);
writefln("\teattr = %03o, %03o", de.externalAttributes >> 16, de.externalAttributes & 0xFFFF);
writefln("\teattr = %03o, %03o", de.fileAttributes);
Copy link
Member Author

Choose a reason for hiding this comment

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

The public fileAttributes does this in the setter

dmd -m$MODEL $file
done
# TODO: fix DustMite
#(cd DustMite && dmd -m$MODEL dustmite.d)
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

dmd -m$MODEL dustmite.d splitter.d

Copy link
Member

Choose a reason for hiding this comment

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

There is a Makefile rule for Dustmite already, why duplicate the build commands here?

}

install_digger() {
$DIGGER build --model=$MODEL "master"
run_digger() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Digger gets installed with build_digger, here we really run it

- add module prefix flag
- allow single files as input as well
- beautify the ddoc comment header parsing
- add check for empty files
- more resistant leading dot/slashes removal
@wilzbach wilzbach force-pushed the add-check-for-public-unittests branch from 36d0c0d to d9762d7 Compare December 15, 2016 17:51
@andralex
Copy link
Member

Will merge now. cc @CyberShadow @MartinNowak @WalterBright

@andralex andralex merged commit 184f5e6 into dlang:master Dec 15, 2016
@wilzbach wilzbach deleted the add-check-for-public-unittests branch December 15, 2016 22:19
@wilzbach
Copy link
Member Author

Will merge now. cc @CyberShadow @MartinNowak @WalterBright

Thanks - I wanted to get the CI working before, but well luckily it does work at Phobos :)
dlang/phobos#4943

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.

None yet

3 participants