Skip to content

Add simple constructive tests without scopes#191

Open
EmmanuelMess wants to merge 8 commits into
SebastianMestre:masterfrom
EmmanuelMess:tests/constructivetests
Open

Add simple constructive tests without scopes#191
EmmanuelMess wants to merge 8 commits into
SebastianMestre:masterfrom
EmmanuelMess:tests/constructivetests

Conversation

@EmmanuelMess

Copy link
Copy Markdown

Very basic initial implementation of constructive tests.

@SebastianMestre SebastianMestre left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Here's an idea: why not make a the fuzzer into a separate executable? I feel like the use cases for it are different than unit tests. (PS: Our build system is a hand-written makefile, so feel free to ask any questions regarding how to achieve this)

Other than that, the implementation looks pretty good, but there are some style things I would really like changed.

Comment thread Makefile Outdated
Comment thread src/test/constructive_test.cpp Outdated
Comment thread src/test/constructive_test.cpp Outdated
if (i == list.end()) {
i = list.begin();
}
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

huh that's a cute way to implement this. I would've used recursion and a stream for output, but this idea of replacing symbols in a list is pretty nice.

Comment thread src/test/constructive_test.cpp Outdated
Comment thread src/test/test_set.cpp Outdated
east const, alphabetical ordering in Makefile,
use sstream for final code creation,
fix C11 style struct construction
@EmmanuelMess

Copy link
Copy Markdown
Author

(PS: Our build system is a hand-written makefile, so feel free to ask any questions regarding how to achieve this)

I am finding quite difficult to read the makefile, I am more accustomed to CMake, and the amount of nesting in the macros is a bit confusing. I'll try to copy the current tests configuration and see how it goes.

@lushoBarlett

Copy link
Copy Markdown
Collaborator

I am finding quite difficult to read the makefile

The makefile is daunting, only @SebastianMestre messed with it, I didn't touch it at all. Right now it's pretty organized so with a little help from him we can separate it into a different executable for sure.

@lushoBarlett

Copy link
Copy Markdown
Collaborator

I will also note that we like snake_case and short names if possible (without sacrificing much understanding).

enum Symbol to enum class, corrected typo, removed unused Symbol::paren
Renamed open_brackets to brackets_open and close_brackets to brackets_close
Better spacing for constants
camel case to snake case
@EmmanuelMess

Copy link
Copy Markdown
Author

I will also note that we like snake_case and short names if possible (without sacrificing much understanding).

Done.

@lushoBarlett

lushoBarlett commented Nov 10, 2020

Copy link
Copy Markdown
Collaborator

I have read the Makefile myself and tried to simulate adding a new executable called run_fuzzer and the way the file is program is structured its pretty simple pattern matching with your eyeballs.

We have two executables, interpreter and tests. Wherever you see both, you need to add your fuzzy version. I'll include the not so intuitive examples as proper examples of my idea

+ FUZZY_DIR := fuzzy
+ FUZZY_ENTRY := main
+ FUZZY_TARGETS := \
    file1 \
    file2

...

  $(INTERPRETER_BIN): $(INTERPRETER_ENTRY_OBJECT) $(INTERPRETER_OBJECTS) $(COMMON_OBJECTS)
  $(TEST_BIN): $(TEST_ENTRY_OBJECT) $(TEST_OBJECTS) $(INTERPRETER_OBJECTS) $(COMMON_OBJECTS)
+ $(FUZZY_BIN): $(FUZZY_ENTRY_OBJECT) $(FUZZY_OBJECTS) $(INTERPRETER_OBJECTS) $(COMMON_OBJECTS)

...

- $(INTERPRETER_BIN) $(TEST_BIN):
+ $(INTERPRETER_BIN) $(TEST_BIN) $(FUZZY_BIN):

You should be able to figure out the rest, if not, please ask.

@EmmanuelMess

EmmanuelMess commented Nov 10, 2020

Copy link
Copy Markdown
Author

I did that, and it failed with an error on creating main.d.
Makefile:

SOURCE_DIR := src
BIN_DIR    := bin

BUILD_BASE_DIR  := build

# modules
CONSTRUCTIVE_TEST := run_constructive_tests
INTERPRETER       := jasperi
TEST              := run_tests

CXXFLAGS := -std=c++14 -Wall
LIBS :=

COMMON_DIR := .
COMMON_TARGETS := \
	algorithms/tarjan_solver \
	algorithms/unification \
	utils/polymorphic_block_allocator \
	utils/polymorphic_dumb_allocator \
	utils/block_allocator \
	utils/interned_string \
	utils/span \
	utils/string_set \
	utils/string_view \
	ast \
	compile_time_environment \
	compute_offsets \
	ct_eval \
	desugar \
	error_report \
	lexer \
	match_identifiers \
	metacheck \
	parse \
	parser \
	token \
	typecheck \
	typechecker \
	typed_ast \
	typesystem

CONSTRUCTIVE_TEST_DIR := constructive_test
CONSTRUCTIVE_TEST_ENTRY := main
CONSTRUCTIVE_TEST_TARGETS := \
	constructive_test \
	test_set \
	tester

INTERPRETER_DIR := interpreter
INTERPRETER_ENTRY := main
INTERPRETER_TARGETS := \
	environment \
	error \
	eval \
	execute \
	garbage_collector \
	gc_ptr \
	native \
	utils \
	value

TEST_DIR := test
TEST_ENTRY := main
TEST_TARGETS := \
	constructive_test \
	test_set \
	tester

ifeq ($(MODE),debug)
  CXXFLAGS += -g -fsanitize=address
  LIBS += -lasan
  BUILD_DIR := $(BUILD_BASE_DIR)/debug
else ifeq ($(MODE),tuning)
  CXXFLAGS += -O2 -g -fno-omit-frame-pointer
  BUILD_DIR := $(BUILD_BASE_DIR)/tuning
else ifeq ($(MODE),dev)
  CXXFLAGS += -O0
  BUILD_DIR := $(BUILD_BASE_DIR)/dev
else
  CXXFLAGS += -O3
  BUILD_DIR := $(BUILD_BASE_DIR)/release
endif

COMMON_OBJECTS            := $(COMMON_TARGETS:%=$(BUILD_DIR)/$(COMMON_DIR)/%.o)
CONSTRUCTIVE_TEST_OBJECTS := $(CONSTRUCTIVE_TEST_TARGETS:%=$(BUILD_DIR)/$(CONSTRUCTIVE_TEST_DIR)/%.o)
INTERPRETER_OBJECTS       := $(INTERPRETER_TARGETS:%=$(BUILD_DIR)/$(INTERPRETER_DIR)/%.o)
TEST_OBJECTS              := $(TEST_TARGETS:%=$(BUILD_DIR)/$(TEST_DIR)/%.o)

CONSTRUCTIVE_TEST_ENTRY_OBJECT := $(BUILD_DIR)/$(CONSTRUCTIVE_TEST_DIR)/$(CONSTRUCTIVE_TEST_ENTRY).o
INTERPRETER_ENTRY_OBJECT       := $(BUILD_DIR)/$(INTERPRETER_DIR)/$(INTERPRETER_ENTRY).o
TEST_ENTRY_OBJECT              := $(BUILD_DIR)/$(TEST_DIR)/$(TEST_ENTRY).o

ALL_OBJECTS := \
	$(COMMON_OBJECTS) \
	$(INTERPRETER_OBJECTS) $(INTERPRETER_ENTRY_OBJECT) \
	$(TEST_OBJECTS) $(TEST_ENTRY_OBJECT) \
	$(CONSTRUCTIVE_TEST_OBJECTS) $(CONSTRUCTIVE_TEST_ENTRY_OBJECT)

DEPS := $(ALL_OBJECTS:%.o=%.d)

TEST_BIN := $(BIN_DIR)/$(TEST)
CONSTRUCTIVE_TEST_BIN := $(BIN_DIR)/$(CONSTRUCTIVE_TEST)
INTERPRETER_BIN := $(BIN_DIR)/$(INTERPRETER)

# UTILS

SHOW_COMMAND := @printf "%-15s%s\n"
SHOW_CXX := $(SHOW_COMMAND) "[ $(CXX) ]"
SHOW_DEPS := $(SHOW_COMMAND) "[ INCLUDE ]"

# TARGET DEFINITIONS
all: $(INTERPRETER_BIN)
.PHONY: all

clean:
	rm -r $(BUILD_BASE_DIR)
.PHONY: clean

constructive_tests: $(CONSTRUCTIVE_TEST_BIN)
.PHONY: constructive_tests

interpreter: $(INTERPRETER_BIN)
.PHONY: interpreter

tests: $(TEST_BIN)
.PHONY: tests

$(TEST_BIN): $(TEST_ENTRY_OBJECT) $(TEST_OBJECTS) $(INTERPRETER_OBJECTS) $(COMMON_OBJECTS)
$(CONSTRUCTIVE_TEST_BIN): $(CONSTRUCTIVE_TEST_ENTRY_OBJECT) $(CONSTRUCTIVE_TEST_OBJECTS) $(INTERPRETER_OBJECTS) $(COMMON_OBJECTS)
$(INTERPRETER_BIN): $(INTERPRETER_ENTRY_OBJECT) $(INTERPRETER_OBJECTS) $(COMMON_OBJECTS)

include $(DEPS)

# RULES

$(TEST_BIN) $(CONSTRUCTIVE_TEST_BIN) $(INTERPRETER_BIN):
	$(SHOW_CXX) $@
	@mkdir -p $(dir $@)
	@$(CXX) -o $@ $^ $(LIBS)

$(BUILD_DIR)/%.o: $(SOURCE_DIR)/%.cpp
	$(SHOW_CXX) $@
	@mkdir -p $(dir $@)
	@$(CXX) $(CXXFLAGS) -c -o $@ $<

$(BUILD_DIR)/%.d: $(SOURCE_DIR)/%.cpp
	@#$(SHOW_DEPS) $@
	@mkdir -p $(dir $@)
	@set -e; rm -f $@; \
	$(CXX) -MM $(CPPFLAGS) $< > $@.$$$$; \
	sed 's,\($(*F)\)\.o[ :]*,$(BUILD_DIR)/$*.o $@ : ,g' < $@.$$$$ > $@; \
	rm -f $@.$$$$

@lushoBarlett

Copy link
Copy Markdown
Collaborator

I see you repeated some files in constructive and test targets (they are identical). Also I don't see your current file structure so I can't tell you exactly what went wrong. Did you create a new folder where you place a new main file and your own constructive test files? You shouldn't need the files in the test folder.

@EmmanuelMess

Copy link
Copy Markdown
Author

Also I don't see your current file structure so I can't tell you exactly what went wrong.

This pull request has the current file structure.

Did you create a new folder where you place a new main file and your own constructive test files?

No, I am trying to get two executables with the same file structure for now.

@lushoBarlett

Copy link
Copy Markdown
Collaborator

If you are doing that then this approach of copying the makefile is not going to work. The idea is that you should be creating a new folder with new files and a new main file. This will also be consistent with our setup.

@EmmanuelMess

Copy link
Copy Markdown
Author

If you are doing that then this approach of copying the makefile is not going to work

Sorry, I don't understand, are files generated wrong by the makefile I showed?

@SebastianMestre

SebastianMestre commented Nov 12, 2020

Copy link
Copy Markdown
Owner

The error is that you are trying to build out of the constructive_test directory, but no such directory exists.

look at this line in your modified makefile:

CONSTRUCTIVE_TEST_DIR := constructive_test

After changing that to

CONSTRUCTIVE_TEST_DIR := test

It builds as expected

Of course, this gives make the exact same information as the test targets, so it just builds the tests with a different name.

@SebastianMestre

SebastianMestre commented Nov 12, 2020

Copy link
Copy Markdown
Owner

I would appreciate it if you could refer to this as "the fuzzer" within the codebase. So, name the executable run_fuzzer, the targets FUZZER_TARGETS, etc., please.

Also, in my experience, a fuzzer is not run for a fixed amount of iterations in CI, but until it crashes, or is stopped by hand, in a developer's (or dedicated) machine. This would be much more desirable behavior.

@SebastianMestre SebastianMestre added area - testing kind - feature New feature or request triage - investigating Extra attention is needed labels Jan 28, 2021
@SebastianMestre

Copy link
Copy Markdown
Owner

I would like to have randomized testing, so I can take over this PR if you are ok with it @EmmanuelMess

@EmmanuelMess

EmmanuelMess commented Feb 4, 2022

Copy link
Copy Markdown
Author

I would like to have randomized testing, so I can take over this PR if you are ok with it @EmmanuelMess

Yes, no problem. Do see how QuickCheck implements arbitrary, and how arbitrary is implemented for AST trees, I am doing something similar to test a command based language for Haskell, will publish it after presenting it as a final project.

AFAIK the idea for QuickCheck is to implement a printer that prints an arbitrary AST, and have the randomizer generate the AST, so: print AST then parse and compare to original AST; if they are different there is a mistake in the parser or the printer, probably the parser.

@SebastianMestre

Copy link
Copy Markdown
Owner

AFAIK the idea for QuickCheck is to implement a printer that prints an arbitrary AST, and have the randomizer generate the AST, so: print AST then parse and compare to original AST; if they are different there is a mistake in the parser or the printer, probably the parser.

Makes sense. I honestly don't care so much about testing the parser for correctness. I'm mainly interested in finding unhandled error conditions, like crashes. So the approach used in this PR would do just fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area - testing kind - feature New feature or request triage - investigating Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants